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

Introduce "navigate" mode #126

Closed
annevk opened this issue Sep 17, 2015 · 10 comments
Closed

Introduce "navigate" mode #126

annevk opened this issue Sep 17, 2015 · 10 comments

Comments

@annevk
Copy link
Member

annevk commented Sep 17, 2015

The reason navigate needs to be "no-cors" is due to the Origin header. We cannot just override the origin to always be same-origin, because that would give the wrong value for the Origin header. And I don't think we want to introduce a second origin concept.

@wanderview
Copy link
Member

Do you have any thoughts on how opaque origins will be blocked for navigations? It seems worker scripts should still be covered by the same-origin RequestMode.

@annevk
Copy link
Member Author

annevk commented Sep 17, 2015

So what I think we should do is that HTML just treats any tainted response as same-origin (that is what happens today and makes sense) and that Fetch blocks opaque responses from the service worker (so that you can't exploit it).

@wanderview
Copy link
Member

It seems clunky to have special cases for navigations that aren't reflected in the exposed FetchEvent.request attributes.

If navigations follow no-cors cross-origin redirects, is there a reason we can't just accept opaque responses for navigations? It seems if we update the final URL of the page with the intercepted Response.url, then it would be the same as a redirect.

Maybe something like:

  • Add an InterceptedUrlMode enum with values "hidden" and "exposed"
  • Request objects default to "hidden" url mode
  • Navigations set url mode to "exposed"
  • If url mode is "exposed", then skip step 15 of main fetch (overriding the returned response.url)

@jakearchibald, is there a functional reason not to do this for navigations?

@wanderview
Copy link
Member

From IRC:

11:52 AM <JakeA> wanderview: because I might respond with caches.match('/static/page-shell-7bfab2c.html')
11:53 AM <JakeA> This is what https://wiki-offline.jakearchibald.com/ does
11:54 AM <JakeA> https://github.com/jakearchibald/offline-wikipedia/blob/master/public/js/sw/index.js#L82

@annevk
Copy link
Member Author

annevk commented Sep 17, 2015

So @wanderview suggested introducing "navigate" as a mode since navigation really has its own policy. We would not expose that to script, just the HTML Standard. The alternative is that security branches on both mode and destination. (Having said that, per #127 it might already end up branching on mode and redirect mode. So I'm not sure how consistent we'd be.)

I wonder what @tyoshino et al think of this.

@wanderview
Copy link
Member

(Having said that, per another suggestion it might already branch on mode and redirect mode. So I'm not sure how consistent we'd be if we did that too.)

  • The thing that determines if you get an opaqueredirect response is RequestRedirect.
  • The thing that determines if you get an opaque response is RequestMode.

In both cases, checking the Response type based on the Request attribute which determines if you get that Response type seems natural to me.

Setting navigation request to "no-cors" RequestMode implies that opaque responses should be allowed. But we don't allow them, because what we really mean is same-origin, but we can't use that because it forces the origin header which isn't right for navigations. None of the current RequestMode values really fits for navigations.

To me that really suggests we need a new RequestMode for navigations.

@annevk
Copy link
Member Author

annevk commented Sep 22, 2015

Okay, so

  1. Add "navigate"
  2. Forbid "navigate" from being set by RequestInit, but do allow it to be copied.
  3. If any of init's members are present and "navigate" was copied, throw.

@annevk annevk changed the title Navigate likely needs to be "no-cors" after all, need to update security checks Introduce "navigate" mode Sep 22, 2015
@annevk annevk closed this as completed in a74b317 Sep 25, 2015
annevk added a commit to whatwg/html that referenced this issue Sep 25, 2015
See whatwg/fetch#126 for context around the
“navigate” value for request’s mode. The other additions were
unfortunately omitted from 7c5555a.
@wanderview
Copy link
Member

Would it be possible to put the "navigate" type at the end of the enumeration list?

We try to keep the webidl a direct copy of whats in the spec, which means the binary enumeration values of the RequestMode would change by putting "navigate" first. This matters because we store these enumeration values in Cache databases.

I can deal with this, but it would be easier if enumeration order wasn't changed. Appending new enumeration values avoids the problem.

@annevk
Copy link
Member Author

annevk commented Sep 26, 2015

The specification doesn't expose order in any way, so it should not matter.

@wanderview
Copy link
Member

annevk added a commit to whatwg/html that referenced this issue Sep 30, 2015
See whatwg/fetch#126 for context around the
“navigate” value for request’s mode. The other additions were
unfortunately omitted from 7c5555a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants