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

clients.get() should not resolve before sandboxing determines the origin of the client #1385

Open
mfalken opened this issue Jan 24, 2019 · 3 comments

Comments

@mfalken
Copy link
Member

mfalken commented Jan 24, 2019

I think this is an oversight in the spec.

clients.get(id) does:
"Wait for either client’s execution ready flag to be set or for client’s discarded flag to be set."
https://w3c.github.io/ServiceWorker/#clients-get

The execution ready flag seems set at:
"7. Set the active document of browsingContext to document."
https://html.spec.whatwg.org/multipage/browsers.html#creating-a-new-browsing-context

Which later on does:
"11. Implement the sandboxing for document."

Probably we must implement the sandboxing for document first, and if the document has a unique origin we "Run the environment discarding steps for reservedEnvironment." like a cross-origin redirect. Then we can set execution ready.

Chrome currently resolves to a WindowClient. Firefox resolves with undefined. I think the spec intent is undefined.

cc @annevk @wanderview

@mfalken
Copy link
Member Author

mfalken commented Jan 24, 2019

Ah, process a navigation response https://html.spec.whatwg.org/multipage/browsing-the-web.html#process-a-navigate-response includes https://html.spec.whatwg.org/multipage/browsing-the-web.html#initialise-the-document-object which does: "Implement the sandboxing for document". So probably here we need to run the discarding steps if the document is sandboxed.

@mfalken
Copy link
Member Author

mfalken commented Jan 24, 2019

Also I thought I saw Firefox passing the test I wrote but it seems to fail after all.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 24, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 915685
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 24, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 24, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
@wanderview
Copy link
Member

So, AIUI your test creates an iframe with a URL and the resulting load sets sandboxing via a CSP header.

I think resultingClientId probably should resolve before the CSP header is applied. Currently its supposed to resolve with the initial about:blank. If the final document is same-origin, then the client is preserved. If it ends up cross-origin due to navigation or header-applied sandboxing, then that initial about:blank client is discarded and a new one created. But before the document is loaded and headers applied, though, the initial about:blank client is execution ready.

So I think this is why firefox is failing your test. Its marking the initial about:blank execution ready which causes it to resolve the client.

Since chrome does not expose initial about:blank in the clients API yet, I think we could perhaps disambiguate the test to avoid the situation. A couple ideas:

  1. Set sandboxing via an iframe attribute instead of a header.
  2. Make the SW script stash the resultingClientId and only do the clients.get() after the iframe has loaded.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 25, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 25, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 25, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 26, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2019
…igin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 5, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Feb 6, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 7, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Feb 8, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderviewchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035

UltraBlame original commit: f8d4418d0e6360b35baf196d1a0f5c0b51d0b944
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderviewchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035

UltraBlame original commit: 493155c6564f7af94eef385c4b4eb04aef303845
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderviewchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035

UltraBlame original commit: f8d4418d0e6360b35baf196d1a0f5c0b51d0b944
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderviewchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035

UltraBlame original commit: 493155c6564f7af94eef385c4b4eb04aef303845
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderviewchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035

UltraBlame original commit: f8d4418d0e6360b35baf196d1a0f5c0b51d0b944
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…sultingClientId) for cross-origin., a=testonly

Automatic update from web-platform-tests
WPT: service worker: test clients.get(resultingClientId) for cross-origin.

See w3c/ServiceWorker#1385

Bug: 924959
Change-Id: I5e2850b743d0702b36f1f20a84c87591c3baab19
Reviewed-on: https://chromium-review.googlesource.com/c/1433657
Reviewed-by: Ben Kelly <wanderviewchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#626265}

--

wpt-commits: 0e0e65d5a43926e584537ea0878b0c2fbc1b694c
wpt-pr: 15035

UltraBlame original commit: 493155c6564f7af94eef385c4b4eb04aef303845
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

No branches or pull requests

2 participants