elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.88k stars 586 forks source link

Weak etags for static assets? #1133

Closed asppsa closed 1 year ago

asppsa commented 1 year ago

Hi there,

Sorry for the long description - basically I am wondering if the default etag generation code in Plug.Static should rightly have a W/ at the beginning to mark the etag as weak.

Context:

We recently noticed at my work that requests for assets served from our Phoenix app's priv/static directory were never getting a 304 response, even when the request had an If-None-Match header which seemed to contain a matching etag. We tracked the issue down to Cloudflare, which in compressing the response using gzip, was also altering the etags served from Phoenix to be weak. This is apparently required by the HTTP spec when an etag doesn't change based on content-encoding - see https://stackoverflow.com/questions/51973120/where-does-the-w-in-an-etag-appear-from.

We can fix our issue by setting up a custom etag_generation tuple that prefixes a "W/", but I was wondering if perhaps the default etag generator in Plug.Static should be weak anyway, given that it the etag is based only on information about the file (size and modification time)?

For what it's worth, this appears to be the conclusion reached by both the Rack and Rails teams.

josevalim commented 1 year ago

Thanks for the report. I am looking at this and it seems that our code is correct? We don't serve different content over the same etag, or am I misreading the docs?

asppsa commented 1 year ago

I'm not an expert, but I think you are probably right - in particular I can see that the etag changes with the content-encoding if brotli and/or gzip are enabled in Plug.Static itself (not using some reverse proxy as we are), which is the example given in https://www.rfc-editor.org/rfc/rfc7232#section-2.3.3. By contrast it seems that Rack does not does not do this.

Thanks for taking the time to look into this - happy to close this issue.

josevalim commented 1 year ago

Closing this but glad to revisit if we are indeed wrong!