Use HTTP 404 & forward Cache-Control, Content-Type, Expires, and Last-Modified headers from S3. #158
Conversation
john-shaffer
changed the title
john-shaffer
changed the title
| @@ -46,6 +46,8 @@ class ImageRequest { | |||
| const request = s3.getObject(imageLocation).promise(); | |||
| try { | |||
| const originalImage = await request; | |||
| this.Expires = originalImage.Expires; | |||
| this.LastModified = originalImage.LastModified; | |||
There was a problem hiding this comment.
I've tried this fix and the s3.getObject() call returns an invalid date format (e.g. 2019-11-06T21:36:00.000z). Added the following to change into a valid HTTP header format.
const formattedLastModified = new Date(originalImage.LastModified).toUTCString();
There was a problem hiding this comment.
Thanks. I noticed this, but hadn't had time to investigate it. That fix is in the pull request now.
|
Thanks for your input. We will consider to change in the next release. |
|
@beomseoklee - How often are releases performed? I'd love to incorporate this change into our platform |
|
@beomseoklee any news about this PR? because |
|
@john-shaffer I think will be great to have original Cache-Control as well in this PR as well p.s. also need fix tests according to these changes |
|
@john-shaffer btw, please take a look changes in my fork, need to add conditions to the dates, if original object doesn't have such headers, client will receive |
|
@vitalinfo I merged your changes and added similar conditions on the other headers so that the tests all pass. |
john-shaffer
changed the title
| @@ -22,9 +22,14 @@ exports.handler = async (event) => { | |||
| const request = await imageRequest.setup(event); | |||
| console.log(request); | |||
| const processedRequest = await imageHandler.process(request); | |||
| const headers = getResponseHeaders(); | |||
| headers["Content-Type"] = request.ContentType; | |||
There was a problem hiding this comment.
What if you are changing the image from a jpg to a png.
If i put
'toFormat' => 'png'
in my image settings but the source is a jpeg this will output image/jpg
| headers["Content-Type"] = request.ContentType; | ||
| headers["Expires"] = request.Expires; | ||
| headers["Last-Modified"] = request.LastModified; | ||
| headers["Cache-Control"] = request.CacheControl; |
There was a problem hiding this comment.
Would it be possible to supply overridden values? In the base64 encoded string
{
"header": [{'Expires' : x }]
}
There was a problem hiding this comment.
Good catch, @bretto36. I'm not using either of those features (changing formats or base64), but I'll take a look when I get a chance.
|
Thanks for your effort here, @john-shaffer You can refer to the recent changes here |
|
Thank you, @beomseoklee. We're very happy with the latest version. |
Fixes #159, #103, and #120.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.