w3c / resource-timing

Resource Timing
https://w3c.github.io/resource-timing/
Other
119 stars 35 forks source link

Add delivery type to PerformanceResourceTiming. #343

Closed jeremyroman closed 1 year ago

jeremyroman commented 1 year ago

Addresses #332.

This defines a concept of "delivery type" which currently describes the difference between resources delivered from the HTTP cache and those not (currently only detectable via transferSize). It is expected to be extended to describe resources delivered from other stores, such as the consuming resources from the HTML preload buffer and navigational prefetches.


Preview | Diff

jeremyroman commented 1 year ago

This is missing WPT tests at present. I understand if merging this is blocked on that, but would you mind having a look @noamr ? I expect the tests for this will be relatively straightforward (at least, once we have an implementation in Chromium to avoid test bugs).

jeremyroman commented 1 year ago

LGTM, but I'd love to run this by the WG and see if there are any objections, as well as get tests in place before this lands.

Is TPAC a good venue for that? If so are there steps I should take to get it on the agenda?

yoavweiss commented 1 year ago

Is TPAC a good venue for that? If so are there steps I should take to get it on the agenda?

We can probably cover this on today's call.

nicjansma commented 1 year ago

Question: Would this attribute be gated by same-origin or TAO/CORS?

For the "cache" status: In RUM we like to know whether resources are cached or not. For same-origin/TAO we utilize transferSize=0 to know this, but for X-O resources we apply a heuristic of "< ~30ms must be cached" (this isn't perfect). If cache is not TAO protected this could give us a more precise knowledge of cache status, but I can understand if this would have to be TAO (or CORS) protected.

Other than that, the other states that this could have for e.g. nav prefetch seem useful.

nicjansma commented 1 year ago

Summary from 9/1 W3C WebPerf call:

yoavweiss commented 1 year ago

Thinking out loud regarding the need for restrictions here: If we don't add any TAO restrictions to this, this doesn't reveal anything about the resource itself, but it can reveal some metadata about the resource. Specifically, it can reveal things about the resource's caching headers. It provides a bit on whether the resource is cachable or not (and potentially, for how long it is cacheable, through continous probing).

While not significant on its own, this could be sufficient to reveal user state in cases where the cachability of the resource varies between e.g. logged-in and non-logged-in users.

So, I think we want this to be TAO protected.

/cc @arturjanc and @mikewest to tell me if I'm being overly paranoid about this or overly optimistic about this not already being leaked through timing.

jeremyroman commented 1 year ago

I think that tracks. I think the strongest counterarguments would be:

  1. The risk is substantially mitigated in browsers which partition the cache by at least top-level site (and some also use a secondary key). In such cases the cache state doesn't reveal anything about browsing activity on other top-level sites. I'm not certain but I believe this is now all major browsers.
  2. It is likely already deducible from timing heuristics (even without TAO-protected info) in most cases.

I'm not opposed to TAO protection, though, and presumably anyone gathering RUM is also serving their important cross-origin resources with a suitable TAO header.

If that is the resolution, the behavior I propose is that the if "cache" would have been the delivery type but the response's timing allow passed flag is unset, then it returns "" instead. This is probably slightly conservative since it would protect it if a redirect didn't pass the TAO check even if the final resource did, but similar concerns might apply to the destination of the redirect and there's value in being consistent with other TAO protected properties.

jeremyroman commented 1 year ago

On further investigation, I think the current text in this PR already implies a TAO check in the following way:

Fetch says:

If response’s timing allow passed flag is not set, then set timingInfo to a the result of creating an opaque timing info for timingInfo, set bodyInfo to a new response body info, and set cacheState to the empty string.

This is then used to populate the cache mode and thus the default delivery type, so "setup the resource timing entry" won't see the "cache" delivery type if the timing allow passed flag wasn't set (i.e., the response didn't pass a TAO check).

jeremyroman commented 1 year ago

Status update:

I believe that covers the identified items. @yoavweiss what is the next step under the group's working mode?

yoavweiss commented 1 year ago

^^ @achristensen07 @bdekoz

jeremyroman commented 1 year ago

369 seems to be general verbiage that I think covers any application it would have to this PR. I'd like to renew my request to know what the next step/milestone here is.

noamr commented 1 year ago

This is LGTM, @yoavweiss do you want to take another look?