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.

Sign up for GitHub

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

Jump to bottom

addRoutes should check for requestMethod #1747

Closed
youennf opened this issue Jan 22, 2025 · 3 comments
Closed

addRoutes should check for requestMethod #1747

youennf opened this issue Jan 22, 2025 · 3 comments

Comments

@youennf
Copy link

youennf commented Jan 22, 2025

Route condition requestMethod is compared to a request method as part of https://w3c.github.io/ServiceWorker/#match-router-condition.
Like done for Request constructor, requestMethod should be:

  1. validated with https://fetch.spec.whatwg.org/#concept-method and https://fetch.spec.whatwg.org/#forbidden-method
  2. normalized with https://fetch.spec.whatwg.org/#concept-method-normalize.
@yoshisatoyanagisawa yoshisatoyanagisawa mentioned this issue Jan 23, 2025
Merged
@yoshisatoyanagisawa
Copy link
Collaborator

sounds good to me.
Let me update the specification like so.
#1748

@yoshisatoyanagisawa
Copy link
Collaborator

Also filed https://issues.chromium.org/issues/391722219 to address this in Chromium.

yoshisatoyanagisawa added a commit that referenced this issue Jan 27, 2025
@yoshisatoyanagisawa
Check requestMethod
2f58e56 
As suggested in #1747,
addRoutes() checks requestMethod is valid, and matching will happens
with a normalized method.
@yoshisatoyanagisawa
Copy link
Collaborator

The specification has been updated to follow the suggestions.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 28, 2025
@yoshisatoyanagisawa @chromium-wpt-export-bot
Check the requestMethod field with addRoutes()
d756856 
As proposed in w3c/ServiceWorker#1747,
let me make the Chromium validate the requestMethod with
https://fetch.spec.whatwg.org/#concept-method and
https://fetch.spec.whatwg.org/#forbidden-method

There is also ongoing spec change in:
w3c/ServiceWorker#1748

Change-Id: I34b4ebb467b7e45bf36d972764c2cfaf8b059019
Bug: 391722219
Merged
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 28, 2025
@yoshisatoyanagisawa
Check the requestMethod field with addRoutes()
8271d15 
As proposed in w3c/ServiceWorker#1747,
let me make the Chromium validate the requestMethod with
https://fetch.spec.whatwg.org/#concept-method and
https://fetch.spec.whatwg.org/#forbidden-method

There is also ongoing spec change in:
w3c/ServiceWorker#1748

Change-Id: I34b4ebb467b7e45bf36d972764c2cfaf8b059019
Bug: 391722219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191305
Reviewed-by: Keita Suzuki <[email protected]>
Reviewed-by: Shunya Shishido <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Commit-Queue: Yoshisato Yanagisawa <[email protected]>
Reviewed-by: Minoru Chikamune <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412047}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 28, 2025
@yoshisatoyanagisawa @chromium-wpt-export-bot
Check the requestMethod field with addRoutes()
d64e235 
As proposed in w3c/ServiceWorker#1747,
let me make the Chromium validate the requestMethod with
https://fetch.spec.whatwg.org/#concept-method and
https://fetch.spec.whatwg.org/#forbidden-method

There is also ongoing spec change in:
w3c/ServiceWorker#1748

Change-Id: I34b4ebb467b7e45bf36d972764c2cfaf8b059019
Bug: 391722219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191305
Reviewed-by: Keita Suzuki <[email protected]>
Reviewed-by: Shunya Shishido <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Commit-Queue: Yoshisato Yanagisawa <[email protected]>
Reviewed-by: Minoru Chikamune <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412047}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 28, 2025
@yoshisatoyanagisawa @chromium-wpt-export-bot
Check the requestMethod field with addRoutes()
0f12913 
As proposed in w3c/ServiceWorker#1747,
let me make the Chromium validate the requestMethod with
https://fetch.spec.whatwg.org/#concept-method and
https://fetch.spec.whatwg.org/#forbidden-method

There is also ongoing spec change in:
w3c/ServiceWorker#1748

Change-Id: I34b4ebb467b7e45bf36d972764c2cfaf8b059019
Bug: 391722219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191305
Reviewed-by: Keita Suzuki <[email protected]>
Reviewed-by: Shunya Shishido <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Commit-Queue: Yoshisato Yanagisawa <[email protected]>
Reviewed-by: Minoru Chikamune <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412047}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 29, 2025
@yoshisatoyanagisawa @moz-wptsync-bot
Bug 1944199 [wpt PR 50325] - Check the requestMethod field with addRo…
31da44a 
…utes(), a=testonly

Automatic update from web-platform-tests
Check the requestMethod field with addRoutes()

As proposed in w3c/ServiceWorker#1747,
let me make the Chromium validate the requestMethod with
https://fetch.spec.whatwg.org/#concept-method and
https://fetch.spec.whatwg.org/#forbidden-method

There is also ongoing spec change in:
w3c/ServiceWorker#1748

Change-Id: I34b4ebb467b7e45bf36d972764c2cfaf8b059019
Bug: 391722219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191305
Reviewed-by: Keita Suzuki <[email protected]>
Reviewed-by: Shunya Shishido <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Commit-Queue: Yoshisato Yanagisawa <[email protected]>
Reviewed-by: Minoru Chikamune <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412047}

--

wpt-commits: 0f129137aac7bdec6c498430b8cde75cc5b325fe
wpt-pr: 50325
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Feb 5, 2025
@yoshisatoyanagisawa @moz-wptsync-bot
Bug 1944199 [wpt PR 50325] - Check the requestMethod field with addRo…
8e9af60 
…utes(), a=testonly

Automatic update from web-platform-tests
Check the requestMethod field with addRoutes()

As proposed in w3c/ServiceWorker#1747,
let me make the Chromium validate the requestMethod with
https://fetch.spec.whatwg.org/#concept-method and
https://fetch.spec.whatwg.org/#forbidden-method

There is also ongoing spec change in:
w3c/ServiceWorker#1748

Change-Id: I34b4ebb467b7e45bf36d972764c2cfaf8b059019
Bug: 391722219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191305
Reviewed-by: Keita Suzuki <[email protected]>
Reviewed-by: Shunya Shishido <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Commit-Queue: Yoshisato Yanagisawa <[email protected]>
Reviewed-by: Minoru Chikamune <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412047}

--

wpt-commits: 0f129137aac7bdec6c498430b8cde75cc5b325fe
wpt-pr: 50325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@youennf @yoshisatoyanagisawa