Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Note in Origin doc how no-cors mode affects GET/HEAD vs POST #4853

Merged
merged 1 commit into from
May 20, 2021

Conversation

hmolsen
Copy link
Contributor

@hmolsen hmolsen commented May 10, 2021

What was wrong/why is this fix needed? (quick summary only)
The Origin header is however added for cross-origin POST requests, even if they are simple POST requests. This becomes clear in Point 3. of the Origin header algorithm in https://fetch.spec.whatwg.org/#append-a-request-origin-header
I have verified this behaviour in both Chrome and Firefox latest.

MDN URL of main page changed
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin

Issue number (if there is an associated issue)

Anything else that could help us review it

The Origin header is however added for cross-origin POST requests, eben if they are simple POST requests. This becomes clear in Point 3. of the Origin header algorithm in https://fetch.spec.whatwg.org/#append-a-request-origin-header
@hmolsen hmolsen requested a review from a team as a code owner May 10, 2021 04:25
@hmolsen hmolsen requested review from mirunacurtean and removed request for a team May 10, 2021 04:25
@hmolsen hmolsen changed the title Update index.html Add that the exception is only true for GET and HEAD methods, POST will always cause an Origin header to be added May 10, 2021
@github-actions
Copy link
Contributor

Preview URLs

Flaws

None! 🎉

External URLs

URL: /en-US/docs/Web/HTTP/Headers/Origin
Title: Origin
on GitHub

No new external URLs

@wbamberg
Copy link
Collaborator

@sideshowbarker , is there any reason not to merge this one?

@sideshowbarker
Copy link
Member

@sideshowbarker , is there any reason not to merge this one?

I guess I’d had in mind to wait on follow-comments from @hamishwillee. But I can guess can go ahead and merge this, and if there’s any further refinement needed, it can be done through a follow-up PR.

@sideshowbarker sideshowbarker changed the title Add that the exception is only true for GET and HEAD methods, POST will always cause an Origin header to be added Note in Origin doc how no-cors mode affects GET/HEAD vs POST May 20, 2021
@sideshowbarker sideshowbarker merged commit c2fbda5 into mdn:main May 20, 2021
@sideshowbarker
Copy link
Member

@hmolsen Thanks much for catching this and fixing it, and congrats on landing your first docs change here — welcome aboard 🎉

@hamishwillee
Copy link
Collaborator

is there any reason not to merge this one?

I guess I’d had in mind to wait on follow-comments from @hamishwillee. But I can guess can go ahead and merge this, and if there’s any further refinement needed, it can be done through a follow-up PR.

@sideshowbarker FYI I really appreciate the consideration, but if you're happy with a PR I am very happy for you to merge and to come back later for follow up. FYI I work on MDN Monday, Tuesday, Friday, and only fly-by on the other days.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants