serverless-image-proxy
serverless-image-proxy copied to clipboard
add ETag to header output
Add ETag to header output based on s3 ETag Attempt to close #13 @schickling can you have a look? Thanks!
@kbrandwijk what do you suggest? I think because the images basically will never change - because if you change the image on graphcool it will be a new file anyways - the hardcoded cache value basically let the file stay in cache more-less forever? I think thats alright in this context.
You are actually right! But if resources never change, what's the use for using ETag? Also, when using ETag, if you would query the server with If-None-Match
, it should return 304
without a body. Right now, it doesn't. So combined with the fact that files never change, and the hard cache-control value, I don't think this is the right way to deal with ETag.
@kbrandwijk I want to get rid of downgrade in PageSpeed and YSlow due to missing Expired or ETag in the imageproxy. So what do you think is the right approach? Only show Last-Modified
?
I was reading through several posts and it seems that IF-None-Match
would still work if the date
is set correctly?
As far as I understand if 304 is the response and the ETag hasnt changed the browser will fetch the correct image and extends the lifetime of the cached image based on the max-age. http://www.benhallbenhall.com/2012/03/http-codes-200-from-cache-304/
If the client has performed a conditional GET request and access is allowed, but the document has not been modified, the server SHOULD respond with this status code. The 304 response MUST NOT contain a message-body, and thus is always terminated by the first empty line after the header fields.
For me this is all new topic so I was reading through many blog posts to make sure doing the right thing.
https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching https://stackoverflow.com/questions/6350384/http-combining-expiration-and-validation-caching
@dohomi You are mostly right. I'm just saying that in order to provide a meaningful implementation of the ETag header, the part that says: 'return a 304' should also be implemented, which isn't part of the implementation right now.
Also, returning the ETag from S3 might not be sufficient, because all the processing (cropping, resizing etc.) happens after the image is retrieved from S3. This would mean that different representations (based on the cropping/resizing arguments) would return the same ETag, which is also not right.
I like the idea of supporting ETag, but I think it should be supported properly, instead of just passing along a header from S3. In my opinion, these things are missing right now:
- Generate a different ETag for different representations
- Respond correctly to If-None-Match and other ETag-related requests (304 and more)
@kbrandwijk you are right, I added the npm etag
module to generate an accurate etag for the body.
To return a proper 304 we would need to extract the right header from the event
. Then doing basically the same modification to the original image and check if the HTTP request ETag equals the generated etag(body)
. Do you know how to read out the headers correctly?
I think I found a way. Could you have a look if that looks alright to you?
@kbrandwijk @schickling I cloned this project and tested the behaviour on my aws account and it worked as expected. Anything I can do to get this merged?