Skip to content

KAFKA-3578: Allow cross origin HTTP requests on all HTTP methods#1288

Closed
Ishiihara wants to merge 3 commits into
apache:trunkfrom
Ishiihara:kip-56
Closed

KAFKA-3578: Allow cross origin HTTP requests on all HTTP methods#1288
Ishiihara wants to merge 3 commits into
apache:trunkfrom
Ishiihara:kip-56

Conversation

@Ishiihara
Copy link
Copy Markdown
Contributor

No description provided.

.header("Origin", origin)
.header("Access-Control-Request-Method", method)
.options();
assertEquals(404, response.getStatus());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 404 seemed odd to me, so I want to make sure I understand why we're seeing it (since I would normally interpret that as an error). It seems that CORS doesn't require that you return a particular HTTP status code, so for the preflighted request it's actually ok if we return any code here. In the case of Jetty, it looks like the CrossOriginFilter's chainPreflight option defaults to true such that the OPTIONS request continues on for normal handling. Since we don't implement OPTIONS for any of our endpoints (and the one under test, specifically), we end up with a 404. Is that correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. This is purely for testing purpose and making sure the Access-Control-Allow-Methods header is properly set.

@Ishiihara
Copy link
Copy Markdown
Contributor Author

@ewencp Addressed review comments. PTAL.

@ewencp
Copy link
Copy Markdown
Contributor

ewencp commented Apr 29, 2016

LGTM, thanks!

@asfgit asfgit closed this in e503273 Apr 29, 2016
@Ishiihara Ishiihara deleted the kip-56 branch May 9, 2016 23:08
gfodor pushed a commit to AltspaceVR/kafka that referenced this pull request Jun 3, 2016
Author: Liquan Pei <liquanpei@gmail.com>

Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>

Closes apache#1288 from Ishiihara/kip-56
mumrah pushed a commit to mumrah/kafka that referenced this pull request Aug 14, 2024
* Use supplied Time instance to PersisterStateManager for fetching time information so that it is easily mocked.
* Make Exponential backoff more configurable by dependency inversion of initial and max timeout ms.
* Fixed big in max retries count.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants