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

Set request header's guard if no-cors #1076

Merged
merged 1 commit into from
Aug 18, 2020
Merged

Set request header's guard if no-cors #1076

merged 1 commit into from
Aug 18, 2020

Conversation

jakearchibald
Copy link
Collaborator

@jakearchibald jakearchibald commented Aug 18, 2020

Fixes #1074.

I think this is all it needs, because otherwise we want the header guard to be "request".


Preview | Diff

@annevk
Copy link
Member

annevk commented Aug 18, 2020

Do we still need 32.4.2 at this point? Perhaps we could run 32.4 in its entirety before 32?

@jakearchibald
Copy link
Collaborator Author

Good point about the guard setting, but I'm less sure about "If this’s request’s method is not a CORS-safelisted method, then throw a TypeError." - are there cases where we have no-cors requests that use methods that we want to pass through, but don't want developers to be able to create themselves?

@annevk
Copy link
Member

annevk commented Aug 18, 2020

Kind of seems like a bug if you can end up in that state as it would bypass the same-origin policy. I don't mind going this way for now though, but we should leave a source comment at least.

@jakearchibald
Copy link
Collaborator Author

I've added a note about it.

@annevk
Copy link
Member

annevk commented Aug 18, 2020

Sorry I explained myself badly. Unless we know that it actually happens I think it should be a comment in the source code for editors of this document. I'm pretty certain that you cannot get no-cors with a method other than GET/POST.

@jakearchibald
Copy link
Collaborator Author

Hopefully I've understood this time 😄

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I'd like @yutakahirano to take a second look just in case.

Copy link
Member

@yutakahirano yutakahirano left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@annevk annevk merged commit 596a524 into master Aug 18, 2020
@annevk annevk deleted the set-header-guard branch August 18, 2020 14:53
@annevk
Copy link
Member

annevk commented Aug 18, 2020

As this fixes a regression no tests or implementation bugs are needed.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 18, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 18, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 19, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 20, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 20, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 21, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 21, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761
Commit-Queue: Nicolas Arciniega <niarci@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800663}
pull bot pushed a commit to Alan-love/chromium that referenced this pull request Aug 21, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761
Commit-Queue: Nicolas Arciniega <niarci@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800663}
stephenmcgruer pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 22, 2020
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761
Commit-Queue: Nicolas Arciniega <niarci@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800663}

Co-authored-by: Nicolas Arciniega <niarci@microsoft.com>
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 27, 2020
…ervice worker, a=testonly

Automatic update from web-platform-tests
Allow range requests to pass through a service worker (#25053)

This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761
Commit-Queue: Nicolas Arciniega <niarci@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800663}

Co-authored-by: Nicolas Arciniega <niarci@microsoft.com>
--

wpt-commits: 868ca3586f9dda74b2bb3b6f6dfb67a13e359090
wpt-pr: 25053
ambroff pushed a commit to ambroff/gecko that referenced this pull request Nov 4, 2020
…ervice worker, a=testonly

Automatic update from web-platform-tests
Allow range requests to pass through a service worker (#25053)

This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761
Commit-Queue: Nicolas Arciniega <niarci@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800663}

Co-authored-by: Nicolas Arciniega <niarci@microsoft.com>
--

wpt-commits: 868ca3586f9dda74b2bb3b6f6dfb67a13e359090
wpt-pr: 25053
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This change implements the following edits to the Fetch spec:
whatwg/fetch#560
whatwg/fetch#1076

Existing web tests cover the new functionality.

Bug: 847428
Change-Id: Ie63704769e99d4b8d26d8903edf4cc4e3466c124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339761
Commit-Queue: Nicolas Arciniega <niarci@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800663}
GitOrigin-RevId: 5da0ed1e65305ab2c6d9de2bb4ce62f159520ba8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

The guard of headers created from no-cors request
3 participants