Plack::Middleware::ETag should generate Weak etag for dynamic (or proxied) content.
amaxcz opened this issue · 5 comments
I use Dancer2 + starman with plack/middlewares. Frontend is Nginx. I need to have ETags for dynamic content. My site is proxied via Cloudflare. Full Etag doesn't works for unknown reason for me (Cloudflare ignores it, probably headers are wrong).. So I'm trying to use Weak..
Quick dirty fix for that is simple patch. I don't know how to implement this properly. Please add support for it.
Please help me, thanks!
--- ETag.pm.orig 2015-07-15 06:03:13.000000000 +0200
+++ ETag.pm 2017-12-07 13:41:27.766232534 +0100
@@ -51,7 +51,7 @@
} else {
my $sha = Digest::SHA->new;
$sha->add( @{ $res->[2] } );
- $etag = $sha->hexdigest;
+ $etag = "W/" . $sha->hexdigest;
}
Plack::Util::header_set( $headers, 'ETag', $etag );
$self->_set_cache_control($headers);
Because this middleware generates ETags based on the hash on the actual content, using Weak tags feels incorrect.
not agree. NodeJS uses weak etags, RoR also
https://blog.bigbinary.com/2016/03/08/rails-5-switches-from-strong-etags-to-weak-tags.html
etc.. it's optional and Requested as a feature from the users. Please add it. It's mandatory for current web.
I don't want fork this middleware part. Simple, please fix it.
OK, you should've provided these resources in the first place.
it's optional and Requested as a feature from the users. Please add it
I'm sorry, but that's not how this thing is supposed to work...
Quick dirty fix for that is simple patch
I don't think it's quick dirty - it looks sufficient. If you open a pull request with that change i will merge it.
Actually, Plack::Middleware::ETag isn't even part of the Plack distribution. Please file a bug report to https://metacpan.org/release/Plack-Middleware-ETag