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

Is URL fragment (#foobar) allowed for URLs? #172

Closed
horo-t opened this issue Apr 9, 2018 · 10 comments
Closed

Is URL fragment (#foobar) allowed for URLs? #172

horo-t opened this issue Apr 9, 2018 · 10 comments

Comments

@horo-t
Copy link
Collaborator

horo-t commented Apr 9, 2018

After the change #170, URLs (request URL, certUrl, and validityUrl) must be absolute-URL string.

If my understanding is correct, absolute-URL string can't contain the fragment.
The fragments is meaningless for certURL and validityUrl.
But I think it is useful for request URL.

cc: @jyasskin @twifkak @nyaxt

@nyaxt
Copy link
Collaborator

nyaxt commented Apr 9, 2018

I think it makes sense to allow requestURL be an absolute-URL string with fragments

@jyasskin
Copy link
Member

jyasskin commented Apr 9, 2018

Hm, my model was that if a signed exchange has a request URL of https://pub.com/page.html then it's authoritative for all fragments inside that page.

I'm assuming that if https://intermediate.com/nytimes.com/foo.sxg has a request URL of https://nytimes.com/foo.html then a fetch of https://intermediate.com/nytimes.com/foo.sxg#fragment will internally redirect to https://nytimes.com/foo.html#fragment. Do we need to fix something for that to work?

If it already works, what's the use case for saying that a signed exchange holds https://nytimes.com/foo.html#fragment but not https://nytimes.com/foo.html or https://nytimes.com/foo.html#other-fragment?

@davidben
Copy link
Collaborator

davidben commented Apr 9, 2018

Note fragments aren't sent to the server in a fetch over the network either. Nor do they figure into HTTP cache keys or anything. Agreed with @jyasskin that including fragments in the signed exchanges doesn't make sense. Having the fragments preserved on sxg "redirects" seems reasonable. It is how redirects normally work.

@horo-t
Copy link
Collaborator Author

horo-t commented Apr 10, 2018

According to RFC 7231, the Location header of redirection response can have the fragment.
https://tools.ietf.org/html/rfc7231#section-7.1.2
So I think the request URL in signed exchange should be able to have the fragment.

@davidben
Copy link
Collaborator

Yes, the location header can have a fragment and, indeed, a signed location header can have one too. But that field is not analogous to this one. It's the request that's relevant, and remember that the fragment is not sent in the request line.

Also remember the signed exchange is signed, presumably offline. Including the fragment in the request wouldn't be useful because then every possible fragment on the page must be signed separately. That's a whole lot of effort for no gain, since the body does not normally vary by fragment.

If you wish to direct someone to a fragment within a signed exchange, the mechanism Jeff proposed makes a lot more sense. Because resources do not vary by fragments and one can freely direct a user to a fragment by including it in the URL, there's no reason for signed exchange matching to incorporate the fragment. Therefore, don't put the fragment in there.

@nyaxt
Copy link
Collaborator

nyaxt commented Apr 10, 2018

Let me retract my reply earlier. I'm convinced with @jyasskin's model.

@horo-t
Copy link
Collaborator Author

horo-t commented Apr 10, 2018

Yes, I agree that https://intermediate.com/nytimes.com/foo.sxg#fragment should internally redirect to https://nytimes.com/foo.html#fragment.

But offline browsing is one of use cases. It is very useful if "Alex can download a file containing a large website (e.g. Wikipedia) with a fragment URL from its origin, save it to transferrable storage (e.g. an SD card)."

I agree that the fragment doesn't need to be signed.
One option would be to have a metadata section in the signed exchange file which is not used to generate the signature.
But I don't think there is a problem that the signed exchange can have request URL with fragment.

@davidben
Copy link
Collaborator

I don't think saving the fragment in the signed exchange format makes a whole lot of sense. The fragment is roughly the scroll position. (Consider your example, Wikipedia. The fragment doesn't tell you what article you are reading. It tells you to jump to a heading in the article.) If a user saves an HTML page today, it doesn't save the fragment.

While I can see uses where one would want to stash it in, say, a bookmark, or in whatever mechanism offline websites get transmitted as, this does not make sense part of the signed exchange format. Rather it should be some external pointer, which would be needed anyway to pick out which page to look at out of a large package. If it were part of the request URI, verifying the signature would become complex, which is a security risk. If it were part of the format, it would carry over into the other use cases, which is a security risk.

This issue should just be closed WontFix.

@jyasskin
Copy link
Member

The offline browsing use case needs bundling in order to be useful. While that use case doesn't mention fragments, bundles will have metadata with a start_url, and that can include a fragment.

@horo-t Does putting the fragment at the bundle level work for the uses you're thinking of, or do they really need it in the signed exchange? If they really need it, could you describe them in more detail?

Also, thanks for bringing up details like this. Even if we decide not to make the change, it's helpful to consider it explicitly.

@horo-t
Copy link
Collaborator Author

horo-t commented Apr 10, 2018

Ah, I didn't know about start_url. Then, we don't need to allow fragments in request URL of the signed exchange.

I submitted a Chromium CL which checks the validity and the fragment of the URLs.
https://chromium.googlesource.com/chromium/src/+/da1e9ae5f918c66f3f35f56b00190f5ed9773405

@horo-t horo-t closed this as completed Apr 10, 2018
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

4 participants