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

Reduced cachebust fingerprint to be more reasonable #2074

Merged
merged 3 commits into from
Jan 18, 2023

Conversation

orlp
Copy link
Contributor

@orlp orlp commented Jan 17, 2023

Using a full 256-bit hash for cachebusting is complete overkill (and a waste of pagesize bytes). It isn't for security, simply to invalidate the browser cache if the file changes.

I've changed it to 80 bits. Since you'll see on average a collision from a collection of n elements after sqrt(n) random samples, this means you'd have to do 2^40 edits on the same file, with the browser caching all of them, before you would load a stale version. That is, even if you edited your CSS file every second and cached all of them for eternity it would still take you ~34865 years before seeing a collision. I think that's plenty.

So instead of this:

<link rel="stylesheet" href="/assets/style.css?h=23d9476e2b2eb5b846066019f1ffecec1be0e80ce171161175b5441699b24ccf">

You would see:

<link rel="stylesheet" href="/assets/style.css?h=23d9476e2b2eb5b84606">

@orlp
Copy link
Contributor Author

orlp commented Jan 17, 2023

Locally the tests pass, so not sure what the CI is up to.

@Keats
Copy link
Collaborator

Keats commented Jan 17, 2023

There is a hardcoded fingerprint in the can_cachebust_static_files test

@orlp
Copy link
Contributor Author

orlp commented Jan 18, 2023

...I made a line less long and now the cargo fmt CI starts complaining? I can't be bothered with this.

@c-git
Copy link
Contributor

c-git commented Jan 18, 2023

Yeah because now it should be part of the line above. It is not long enough to warrant a separate line

Copy link
Collaborator

@Keats Keats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry about the CI failure, I'll fmt it later

@Keats Keats merged commit 90e27c1 into getzola:next Jan 18, 2023
Keats pushed a commit that referenced this pull request Feb 16, 2023
* Reduced cachebust fingerprint to be more reasonable.
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

Successfully merging this pull request may close these issues.

3 participants