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

Request.destination for <link rel=prefetch> #658

Closed
yoavweiss opened this issue Jan 8, 2018 · 22 comments
Closed

Request.destination for <link rel=prefetch> #658

yoavweiss opened this issue Jan 8, 2018 · 22 comments

Comments

@yoavweiss
Copy link
Collaborator

As mentioned in w3c/resource-hints#66, there's no current Request.destination defined for prefetched resources. We should probably define such a destination, to be able to:

@annevk
Copy link
Member

annevk commented Jan 8, 2018

Why does it need to be different from fetch() et al?

@annevk
Copy link
Member

annevk commented Jan 8, 2018

Maybe using initiator would be better if the purpose is just a unique CSP directive?

@mikewest
Copy link
Member

mikewest commented Jan 8, 2018

It seems reasonable to me that we'd continue using destination in cases where we know how the data will be used: as provides this information, so when it's present, we should use it.

If we don't have that information, I'd be fine either doing the same thing we do for preload without as (which I think defaults to fetch, and therefore sits under connect-src), or defining a new initiator and directive to match (prefetch-src). I don't have a strong opinion about that, so I'm happy to defer to you folks who own that feature (though, if we do the latter, perhaps it should control preload without as as well?).

@yoavweiss
Copy link
Collaborator Author

Why does it need to be different from fetch() et al?

Presumably, a separate destination would enable SW code to know this fetch is speculative and destined for the next navigation. At the same time, I'm not aware of a concrete use-case for that.

Maybe using initiator would be better if the purpose is just a unique CSP directive?

For CSP that would work as well.

@yoavweiss
Copy link
Collaborator Author

The as attribute for prefetch is not implemented anywhere, and we're not likely to introduce it without breaking existing preloads, so I suspect it will go away :/

preload without as is no longer a thing - as is now mandatory for preload (too many people getting it wrong and double downloading). We create the "potential destination" concept that converts the "fetch" as value into the empty string destination.

I'm fine with folding prefetch onto the empty string destination and doing CSP using a separate initiator, at least until a strong use-case for a separate destination emerges.

@annevk
Copy link
Member

annevk commented Jan 8, 2018

prefetch is such a confusing name for "next navigation response", it gets me every time.

Will as ever be used for that? It seems like the destination would always be "document"?

@yoavweiss
Copy link
Collaborator Author

prefetch is not necessarily the next nav response, but is a resource destined for the next navigation (could also be its CSS, JS, etc)

@annevk
Copy link
Member

annevk commented Jan 8, 2018

I see, in that case we should leave the (destination) default as the empty string. I think initiator is the way to go to distinguish it as prefetch. (We could also add a separator initiator for preload if desired.)

If service workers want to know this information at some point we could expose initiator to script, though we should not do so until we've carefully scrutinized it as the existing values are limited to what we needed for specifications only.

@mikewest
Copy link
Member

mikewest commented Jan 8, 2018

I continue not to understand why we have both preload and prefetch, and not to understand why they behave differently. But, assuming that we have them and there are good reasons, and they're not going away, it would be really excellent if y'all could take some time to explain their functionality in terms of Fetch so that the CSP integration just clearly falls out of that. :)

@yoavweiss: Can I ask you and @igrigorik to define some initiator values that make sense for the kinds of pre-whatever things we have lying around that go through Fetch? I don't have strong opinions about what they should be or how they should interact with each other, I just want something CSP can point to. :)

@yoavweiss
Copy link
Collaborator Author

@mikewest sure! I believe preload is well defined, and I don't think we need a specific initiator value for it.

prefetch is indeed not-well-defined atm, which is why I opened this issue :) I can PR Fetch to add a new initiator for it (that CSP can hang off of), and add the definition to Resource Hints to use that initiator, as well as the empty string destination.

OTOH, if we would eventually need to expose it to users, a new destination might be cleaner than exposing initiator IMO.

@mikewest
Copy link
Member

mikewest commented Jan 8, 2018

@yoavweiss: Great! Some of these might be less Fetchy than others, but what about dns-prefetch, preconnect, prerender, modulepreload, and any of the other pieces of LinkRelAttribute::LinkRelAttribute() that route through Fetch? Are they defined in reasonable ways?

@annevk
Copy link
Member

annevk commented Jan 8, 2018

preload is not well-defined when it comes to Fetch integration... In particular the cache situation is rather unclear.

@yoavweiss
Copy link
Collaborator Author

what about dns-prefetch, preconnect, prerender, modulepreload

dns-prefetch and preconnect - do we want to block them using CSP or other means? Do they need to be integrated to Fetch?

prerender and next - I'm under the impression that they were unshipped in Blink and not necessarily implemented anywhere else. If that's the case, we can probably remove them from the spec.

modulepreload - tbh, I haven't followed that up too closely, but I believe @domenic was leading the charge on that definition, as part of HTML.

preload is not well-defined when it comes to Fetch integration... In particular the cache situation is rather unclear.

Yes, the caching situation is not well defined, similarly to the rest of the platform's caching. I agree that Someone™ should define that. I also think that properly defining it (while accounting for the current implementation differences) is a fairly large project. Personally, I'm afraid I won't be able to tackle in the near future.

@mikewest
Copy link
Member

mikewest commented Jan 8, 2018

dns-prefetch and preconnect - do we want to block them using CSP or other means? Do they need to be integrated to Fetch?

Fetch defines various connection concepts: https://fetch.spec.whatwg.org/#connections, so preconnect probably falls in here somewhere (though not as a request, but as part of the "obtain a connection" algorithm.

It's not clear to me whether Fetch wants to talk about DNS. If not, dns-prefetch falls somewhere else.

It seems likely that folks who care about exfiltration would be interested in restricting both, as they clearly communicate to third-parties. I'd be fine with treating both as connect-src, though, so I don't think we need significant new conceptual definitions.

prerender and next

I've lost track of both of these, honestly. If we've unshipped them, would you mind removing the code? :)

modulepreload

If it's in HTML, I think I can safely assume that it's setting properties correctly.

@tomayac
Copy link
Contributor

tomayac commented Jan 8, 2018

I think the use cases I have listed in #634 might equally qualify for the prefetch case. I’m not sure if prefetches while offline (that by default would fail, but could be handled from within a Service Worker) are an actual thing.

@yoavweiss
Copy link
Collaborator Author

@tomayac They could be, but how would you treat prefetches in this case differently than say, regular fetches?

@tomayac
Copy link
Contributor

tomayac commented Jan 8, 2018

@yoavweiss One idea for a news publisher site might be to dynamically soft-fail most-viewed-article-type prefetches with a generic offline page.

@yoavweiss
Copy link
Collaborator Author

@tomayac I'm not sure I get the use case, and why would prefetched resources treated different than regular fetches in this case. If the prefetched resource is in Cache, is there a reason not to serve it? And if it's not in Cache, is there something different to be done between prefetched and non-prefetched resources?

@domenic
Copy link
Member

domenic commented Jan 8, 2018

modulepreload is exquisitely well-defined, if I may say so myself ;). It just reuses the script type=module fetching machinery.

@samuelhorwitz
Copy link

samuelhorwitz commented Jan 8, 2018

dns-prefetch definitely should be blockable by CSP rules, if CSP is going to expand into prefetch data exfiltration prevention, see here for a proof of concept: https://blog.compass-security.com/2016/10/bypassing-content-security-policy-with-dns-prefetching/

The attack is simply http://leakeddata.evildomain.com leaking secret data to the DNS server logs.

There already appears to be a non-standard header X-DNS-Prefetch-Control (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-DNS-Prefetch-Control). This however is a boolean which doesn't allow CSP-style fine tuning and I'm not really sure where it's even actually supported.

Edit: It appears that at least Chrome (http://dev.chromium.org/developers/design-documents/dns-prefetching) and Firefox (http://bitsup.blogspot.com/2008/11/dns-prefetching-for-firefox.html) support the X-DNS-Prefetch-Control header. However the blog post about Firefox claims the value can be switched on and off by introducing new <meta> tags however it's from 2008 and not and official FF resource. The Chromium blog explicitly says this behavior is not allowed and once turned off, it cannot be turned back on on that page.

@tomayac
Copy link
Contributor

tomayac commented Jan 11, 2018

@yoavweiss Sorry, I was over-complicating things. You're right, you could still deal with the current network situation once the actual fetch happens, ignoring what may have happened during the prefetch.

@annevk
Copy link
Member

annevk commented Apr 18, 2018

For clarity, dns-prefetch and preconnect concerns are tracked by w3c/webappsec-csp#282 and #683.

I filed w3c/resource-hints#75 on X-DNS-Prefetch-Control.

This issue will be resolved once #659 lands. Additional review of that appreciated.

If there's anything else remaining here I strongly encourage you to file a new issue. It's rather crowded already.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 26, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
annevk pushed a commit that referenced this issue Apr 27, 2018
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 27, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 27, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 27, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 2, 2018
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 2, 2019
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <yoavyoav.ws>
Reviewed-by: Charlie Harrison <csharrisonchromium.org>
Reviewed-by: Yutaka Hirano <yhiranochromium.org>
Cr-Commit-Position: refs/heads/master{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657

UltraBlame original commit: b60142c1cc9a01efbb27b591c1828015ffa0a6cb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <yoavyoav.ws>
Reviewed-by: Charlie Harrison <csharrisonchromium.org>
Reviewed-by: Yutaka Hirano <yhiranochromium.org>
Cr-Commit-Position: refs/heads/master{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657

UltraBlame original commit: b60142c1cc9a01efbb27b591c1828015ffa0a6cb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <yoavyoav.ws>
Reviewed-by: Charlie Harrison <csharrisonchromium.org>
Reviewed-by: Yutaka Hirano <yhiranochromium.org>
Cr-Commit-Position: refs/heads/master{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657

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

6 participants