Add Cache-Control to rustdoc pages

[jsha]

For /latest/ URLs, set max-age=0. For versioned URLs max-age=10 minutes and stale-while-revalidate=2 months.

The idea behind this is that versioned URLs change mostly in minor ways - the "Go to latest" link at the top, and the list of versions in the crate menu. And setting a long cache time (either via max-age or via stale-while-revalidate) allows pages to be loaded even while offline.

We could probably apply a long stale-while-revalidate to /latest/ URLs as well, but this is more likely to have a user-noticeable impact, and the /latest/ URLs are relatively new so we don't want to create any confusing interactions.

Part of #845

[jyn514]

@syphar would you like to review this? I know you've had concerns about caching.

[syphar]

@jsha thanks for the initiative! Would stale-while-revalidate also be respected by CloudFront? I know fastly does, and this might open some more questions.

I think this (+ #845 and also adding this to /latest/) boils down to the question if we would accept some UX inconsistencies for the sake of speed, which is more a general design question.

From the top of my head:

1. a cached rustdoc page might include a "go to latest version" link that leads to a page which is not the latest version, and that page perhaps not showing a "go to latest version" link even when it's not.
2. a cached page might not include all released versions, or yanks in the version dropdown
3. the warning of the requested version being yanked won't be visible when accessing the cached version, only after refresh.

I remember a discussion with pietro about caching where these things being always up-to-date was quite important. Of course we can discuss different goals.

In #1552 this would have been solved by not caching any rustdoc pages in the browser but only in the CDN, which we actively invalidate on releases.

[jsha]

Would stale-while-revalidate also be respected by CloudFront?

From my research at https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/Expiration.html, it looks like no. That page doesn't specifically mention stale-while-revalidate but I take that as meaning it's not respected. Note that I am setting max-age=10 minutes in addition to stale-while-revalidate. If that's a concern at CloudFront, we could set that to 0, or we could set s-maxage to 0 (which only affects shared caches).

if we would accept some UX inconsistencies for the sake of speed, which is more a general design question.

Also for the sake of a very valuable feature (#845): being able to view previously visited pages offline. And if we ask the question "would we accept out of date UX if we are offline," I think the answer is definitely yes. The user knows the content will be out of date because they're offline, but keeping access to it is more valuable than losing access because a new crate version might have been uploaded while they were offline.

a cached rustdoc page might include a "go to latest version" link that leads to a page which is not the latest version, and that page perhaps not showing a "go to latest version" link even when it's not.

With #1562 that won't be an issue, since the "go to latest version" will go through a redirect that sends you to /latest/.

a cached page might not include all released versions, or yanks in the version dropdown

I think that's okay for a few reasons. Most actions on docs.rs involve reading the docs, not opening the dropdown, so it would be rare for someone to see a stale list of versions in the dropdown. And docs.rs isn't the canonical up-to-date source for whether there are newer versions available, or which versions are yanked - crates.io is. The most important property of yankedness, of course, is what cargo does at the command line.

the warning of the requested version being yanked won't be visible when accessing the cached version, only after refresh.

True, though if the user does any significant browsing they are likely to wind up visiting a page twice, which would result in getting the updated version, because it was revalidated after the first visit to that page.

Also worth noting that the current system doesn't show the most up-to-date possible information. For instance, when a new release is in the build queue, that doesn't trigger the "Go to latest version" link on the previous version (because there would be nowhere to go), and the dropdown doesn't show the new version yet. Also, users may have a doc page loaded in their browser for arbitrarily long, during which time the "latest" info and the dropdown list of releases may become out of date.

[syphar]

Would stale-while-revalidate also be respected by CloudFront?

From my research at https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/Expiration.html, it looks like no. That page doesn't specifically mention stale-while-revalidate but I take that as meaning it's not respected. Note that I am setting max-age=10 minutes in addition to stale-while-revalidate. If that's a concern at CloudFront, we could set that to 0, or we could set s-maxage to 0 (which only affects shared caches).

Thanks for the clarification! For me a 10 minute cache is totally fine, and typically even takes quite much load from the origin server.

if we would accept some UX inconsistencies for the sake of speed, which is more a general design question.

Also for the sake of a very valuable feature (#845): being able to view previously visited pages offline. And if we ask the question "would we accept out of date UX if we are offline," I think the answer is definitely yes. The user knows the content will be out of date because they're offline, but keeping access to it is more valuable than losing access because a new crate version might have been uploaded while they were offline.

@jsha please correct me if I'm wrong, but stale-while-revalidate would lead to all requests being served from an existing local cache, not only when I'm offline. stale-if-error is perhaps more fitting, though I don't know about the browser support (it seems questionable).

a cached rustdoc page might include a "go to latest version" link that leads to a page which is not the latest version, and that page perhaps not showing a "go to latest version" link even when it's not.

With #1562 that won't be an issue, since the "go to latest version" will go through a redirect that sends you to /latest/.

That is what I assumed, thanks!

a cached page might not include all released versions, or yanks in the version dropdown

I think that's okay for a few reasons. Most actions on docs.rs involve reading the docs, not opening the dropdown, so it would be rare for someone to see a stale list of versions in the dropdown. And docs.rs isn't the canonical up-to-date source for whether there are newer versions available, or which versions are yanked - crates.io is. The most important property of yankedness, of course, is what cargo does at the command line.

👍

the warning of the requested version being yanked won't be visible when accessing the cached version, only after refresh.

True, though if the user does any significant browsing they are likely to wind up visiting a page twice, which would result in getting the updated version, because it was revalidated after the first visit to that page.

Also worth noting that the current system doesn't show the most up-to-date possible information. For instance, when a new release is in the build queue, that doesn't trigger the "Go to latest version" link on the previous version (because there would be nowhere to go), and the dropdown doesn't show the new version yet. Also, users may have a doc page loaded in their browser for arbitrarily long, during which time the "latest" info and the dropdown list of releases may become out of date.

👍

Regarding the design-question: Personally I'm more on the side that outdated docs for a short amount of time (like 30 minutes max) is totally fine. I remember @pietroalbini wanting it always up-to-date, and about @jyn514 I'm not sure.
I would like to have their opinion on the question too.

[jyn514]

I think being outdated for a short time is fine; as long as build.json and the release pages are never cached, it's ok for the documentation pages themselves to be a little outdated. It seems pretty rare to want to navigate to another specific version when you're already on a page, so this is just the difference between the "go to latest" popup showing up or not.

[jsha]

@jsha please correct me if I'm wrong, but stale-while-revalidate would lead to all requests being served from an existing local cache, not only when I'm offline. stale-if-error is perhaps more fitting, though I don't know about the browser support (it seems questionable).

Yes, for pages with stale-while-revalidate (in this PR, the explicitly-versioned pages) each request will be served from the existing local cache, while the browser updates the cache in the background. So for instance if you navigate away and back, or load the page a second time, you'll have a fresh copy.

[syphar]

For the record, I manually tested if the header is actually used,

in firefox:

In chrome I couldn't make it work, though I'm perhaps missing something.

One thing that I just remembered:
In a discussion about caching rustdoc pages on the CDN level CSP was brought up as a reason why these should not be cached. Not having good knowledge about CSP I assume this is still true for the CDN level, but I don't know if this also rules out local caching of rustdoc pages?

If it's only an issue for the CDN we could set max-age=0 to rule that out.

[syphar]

Though it could also be fine for CSP when the nonce only changes every 10 minutes? I'm not sure

[jsha]

In a discussion about caching rustdoc pages on the CDN level CSP was brought up as a reason why these should not be cached. Not having good knowledge about CSP I assume this is still true for the CDN level, but I don't know if this also rules out local caching of rustdoc pages?

Two questions: would caching+CSP block things that should work, and would it allow things that should be blocked? A little background:

An example of XSS is when a site has some query parameter (or other input) that gets interpolated into the page without proper escaping. For example if https://example.com/?name=<script>alert(1);</script> caused an alert to pop up.

CSP aims to stop XSS by saying "I already know all the scripts that will be on my page, so don't run any others." The most desirable CSP policy says "don't run any inline scripts (because that's usually how XSS manifests), and only run scripts that are hosted on my domain."

Unfortunately that's not practical in a lot of cases. Many sites, like docs.rs, have inline scripts and aren't in a position to quickly get rid of them. How do you mark your inline scripts safe without marking the attacker's injected script as safe? You set a nonce in the CSP header and use the same nonce in your <script> tags. So the attacker might try to guess, like https://example.com/?name=<script nonce="2726c7f26c">alert(1);</script> But if the header doesn't match - for instance it might be content-security-policy: default-src 'none'; script-src 'nonce-wMX1quPD' - then the script won't run.

So what happens when the response is cached? The CSP header (with its nonce) will be cached with it. The cached page contains matching nonces on the appropriate scripts, so the appropriate scripts will continue to run on each page load. The "nonce" is now predictable to the attacker for ten minutes, which is a potential problem. They could start sending requests to https://example.com/?name=<script nonce="wMX1quPD">alert(1);</script> now that they know the cached nonce. But those requests wouldn't hit the cache, because it's a new URL. I can imagine scenarios where the protection is weakened - for instance if the XSS was purely in JS rather than server side templating - but I think for the most part they would still require a new URL that would be uncached. Maybe one example is if there were an XSS in the Rustdoc search functionality (which happens on the client side), and we enabled some code in CloudFront to disregard the query part of the URL (?...) when caching.

So, overall my take is: I think it's fine. Also, we don't set CSP headers on rustdoc pages (though I'm guessing we'd eventually like to). Also, the nonces approach is not terrific; IMO it would be better to get rid of inline scripts and use hashes to identify the acceptable versions of main.js, etc.

[jsha]

Any further thoughts on this? To shorten the above: I think there's no interaction between this and CSP because we don't currently set CSP on Rustdoc pages. And I don't think deploying this gets in the way of a future deployment of CSP on Rustdoc pages.

[syphar]

I'm sorry for my recent lack of responses here.

The only thing I don't feel know enough about to approve is probably the combined CSP&caching part.

I'm not sure who in the team knows more? ( @jyn514 ? @Nemo157 ? )
I remember also Pietro saying that caching is not possible with CSP, but I don't know why.

[jsha]

The only thing I don't feel know enough about to approve is probably the combined CSP&caching part.

Note that there isn't really a combined CSP&caching part here. The rustdoc pages this affects don't actually serve a CSP header.

[syphar]

@jsha short update here: since we're changing caching-related things I prefer going a safe (and slowly increasing) approach here (I admit I'm definitely stigmatized by caching/CDN related issues over the last >20 years).

This means two things:

• the expiration time should be a config variable, same as stale-while-revalidate. This enables us to quickly change these values when needed
• I can merge and deploy this when I also have access to invalidate caches in the CDN.

[jyn514]

I can merge and deploy this when I also have access to invalidate caches in the CDN.

I remember @pietroalbini set me up with access to this a while ago, I think https://forge.rust-lang.org/infra/docs/aws-access.html is the relevant page.

[syphar]

I can merge and deploy this when I also have access to invalidate caches in the CDN.

I remember @pietroalbini set me up with access to this a while ago, I think https://forge.rust-lang.org/infra/docs/aws-access.html is the relevant page.

Yeah, let's see if it's this, even better would be extended permissions for our access key, so we can build invalidation into the app.

[jsha]

Thanks for the update @syphar! That's a nice sensible approach, and I definitely understand your caution around caching issues. I'll update the PR to make those things into config variables.

[syphar]

@jsha I now have the access we need and we can continue on this PR, when you find the time.

Since we didn't test this behind CloudFront I would still like to have both settings in config variables and slowly increase them in production.

Now that I think about it, settings should be optional and the header should be omitted when the settings are missing.

[jsha]

Sorry for the delay! Pushed an update with the config options.

[syphar]

I want to start pushing the caching topics again,

@jsha is there anything new that needs to be added here?

Otherwise I would continue test, merge & deploy.

I'll also dig through other pieces where we could add caching following similar rules (/latest/), under the premise we'll run a cdn purge of /krate/* after each release.

[jsha]

This should be good to go! Thanks for pushing it forward.

[syphar]

manual test is fine, we will slowly start and set & watch these settings and their effect.