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

Add quotes to etags #220

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Add quotes to etags #220

merged 1 commit into from
Nov 28, 2024

Conversation

zarqman
Copy link
Contributor

@zarqman zarqman commented Nov 20, 2024

This PR fixes etag generation in Propshaft::Server.

In contrast to other headers, etags must be quoted. (See RFC 9110 §8.8.3.)

Before:
etag: fa74dbac
After:
etag: "fa74dbac"

Etags must be quoted (reference: RFC 9110 §8.8.3)
Before:  etag: fa74dbac
After:   etag: "fa74dbac"
@brenogazzola brenogazzola merged commit 1cfc401 into rails:main Nov 28, 2024
3 checks passed
@brenogazzola
Copy link
Collaborator

Not sure if we should add a W/ before to make it a weak tag, but we can discuss this later. Merged.

@zarqman
Copy link
Contributor Author

zarqman commented Dec 4, 2024

My understanding is that strong etags are appropriate when:
a) the etag is always assured to change if/when the content changes, and
b) the strong etag is only used for a single encoding of the content (identity, gzip, brotli, etc).

In the case of (b), strong etags may still be used with different encodings as long as the etag itself also changes. With digest-based etags, that can be as simple as adding a suffix, like "originalDigest-gzip", although a fresh digest of the compressed content would be valid too (it's just hard to match a fresh digest back to the original if needed).

If an etag is reused across encodings, then it should be weak (or converted to weak). As a general rule, this conversion should be the responsibility of whatever's transforming the encodings.

Strong etags can allow for content to be cached more often or for longer, and are desirable for things like static assets.

Since propshaft doesn't involve itself with compression or any other alternate encodings, and since the digest-sourced etag will definitely change if the content changes, a strong etag seems appropriate.

@zarqman zarqman deleted the quote-etag branch December 4, 2024 23:37
@brenogazzola
Copy link
Collaborator

Your final paragraph addresses the thing I had been wondering: CDNs that automatically serve optimized versions of image files.

I agree that since it’s not Propshaft making that change, then Propshaft can use strong tags and the CDN can handle their part.

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