-
Notifications
You must be signed in to change notification settings - Fork 535
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
Support serving images under s3 subdirectories, Fix to make /fit-in/ work; Fix for VipsJpeg: Invalid SOS error plus several other critical fixes. #130
Conversation
I have manually added this solution to our lambda-function and can confirm that it performs as defined. |
@rpong, images without resizing still doesn't work, example: |
Been trying to get this going - everything seems to work fine, except it's not resizing the image when in a subdirectory - example url: |
…follow exif-specified rotation by default
sorry, my bad. this should be fixed now. |
can you attach the actual photo and actual filename? we discovered an anomaly with filenames containing an 'x' and digits around it causing improper resizing, we will be applying the fix for the PR too in a bit, just want to make sure about your use case and perhaps cover it with our fix later today. thanks man.. |
in my use case, some of key path contains 'x' characters, and i discovered dimension path checking on Line 39
Please add it to your PR, thank you :) |
that work, thanks ! |
also, in previous version (thumbor), i have some use case that unfortunately unsupported on current version:
parseRequestType(event) {
const path = event["path"];
// ----
const matchDefault = new RegExp(
/^(\/?)([0-9a-zA-Z+\/]{4})*(([0-9a-zA-Z+\/]{2}==)|([0-9a-zA-Z+\/]{3}=))?$/
);
const matchThumbor = new RegExp(
/^(\/?)((fit-in)?|(filters:.+\(.?\))?|(unsafe)?).*$/ // Remove extension on this regex
);
const matchCustom = new RegExp(/(\/?)(.*)(jpg|png|webp|tiff|jpeg)/);
const definedEnvironmentVariables =
process.env.REWRITE_MATCH_PATTERN !== "" &&
process.env.REWRITE_SUBSTITUTION !== "" &&
process.env.REWRITE_MATCH_PATTERN !== undefined &&
process.env.REWRITE_SUBSTITUTION !== undefined;
// ----
if (matchDefault.test(path)) {
// use sharp
return "Default";
} else if (matchCustom.test(path) && definedEnvironmentVariables) {
// use rewrite function then thumbor mappings
return "Custom";
} else if (matchThumbor.test(path)) {
// use thumbor mappings
return "Thumbor";
} else {
throw {
status: 400,
code: "RequestTypeError",
message:
"The type of request you are making could not be processed. Please ensure that your original image is of a supported file type (jpg, png, tiff, webp) and that your image request is provided in the correct syntax. Refer to the documentation for additional guidance on forming image requests."
};
}
}
parseImageKey(event, requestType) {
if (requestType === "Default") {
// Decode the image request and return the image key
const decoded = this.decodeRequest(event);
return decoded.key;
} else if (requestType === "Thumbor" || requestType === "Custom") {
// Parse the key from the end of the path
const pathParts = event["path"].split("/");
for (let i = 0; i < pathParts.length; i++) {
if (
pathParts[i] !== "" &&
pathParts[i] !== "fit-in" &&
pathParts[i].match(/^\d+x\d+$/) == null &&
pathParts[i].match(/^filters:/) == null &&
pathParts[i].match(/^smart$/) == null // add this line
) {
const parts = pathParts.slice(i);
return parts.join("/");
}
}
}
// Return an error for all other conditions
throw {
status: 400,
code: "ImageEdits::CannotFindImage",
message:
"The image you specified could not be found. Please check your request syntax as well as the bucket you specified to ensure it exists."
};
}
maybe you have more clean workaround for this problem |
…h digits surrounding it eg: 8x8
Nice! thank you for this, I added this to the PR but moved it outside the foreach loop so it is processed/looked up only once. Just updated the PR as well so the tests pass and several other critical things that required us to fix before we launched our beta app this morning. |
…h digits surrounding it eg: 8x8
Still not working for some of my URLS such as the following: |
https://i.ibb.co/v4RRc5h/Screen-Shot-2019-07-28-at-4-35-55-PM.png thanks for that use case, just pushed a change to the PR which should take care of it, strange that the unit-tests missed this. |
Hi @rpong Thanks for all your hard work. Another issue I just spotted is when you form a URL like so... Specifying "400x" with the old Thumbor used to give you the best fit for 400 and figure it out itself. However, doing that now will give you "NoSuchKey". If you use 400x400 for example then it works fine. |
Hi @adam91holt , noted on this, for now, if this is an actual production use case for you, what we do to circumvent around this is to specify zero and a fit-in. I did not realize they support 400x before, such dimension convention also wasn't in the unit tests so we did not cover that. |
This PR resolves some major BC issues for me. Is there an expected time that we can see this merged back into the main repo? |
Any news on this PR making it in? |
I've tried implementing this manually by exporting the existing Lambda function code, making the changes to the files and then re-uploading a newly zipped version of the function but I'm getting the following error:
Any ideas? |
easier to just use this pre-built one: #130 (comment) goodluck and let us know. |
Any updates? |
Worked very well for me in combination with #157 |
@rpong Thanks for your contribution. But there is a problem: if the filename contains square brackets, the regular expression does not work. Example: [test].jpg does not work (throw error 400) |
Glad to help, btw just released the same package we use with Node 12.x -> https://github.com/innovationlove/serverless-image-handler/releases/tag/4.0.3 Goodluck. |
Will look into this within the week. |
@rpong Thank you very much, again! But I don't think it's working properly. How do you crop an image to exactly a specified size? Before upgrading, if I didn't use the fit-in argument, the image auto-cropped and auto-resized to be EXACTLY the specified size. Now domain.com/fit-in/50x50/image.jpg and domain.com/50x50/image.jpg produces the same result :-( |
Why in this case It doesn't work with lambda which runs on node.js 12.x, but I have the older Serverless Image Handler that runs the lambda on node.js 8.10 and this one works properly |
Got also some problems with the newest version 4.1. Is it still possible to deploy version 4 and then upgrade the node version with the release from innovationlove? |
|
Thanks for your effort here, @rpong You can refer to the recent changes here |
@rpong this solution is not working for image size more thane 4MB . |
I think this is more a Aws Lambda problem because the request and response size is limited: |
Is there a way to use the old |
@giovannipds the old fit-it is working on the last release. Just update your stack with the latest template. |
@cxrod seriously? I probably have edited the code before putting the stack up the last time I've updated here (29 days ago). |
@cxrod I won't re-do this right now since it's already working as I need on my stack 😆 but thanks for letting me know. |
@giovannipds sorry for the late reply! I was observing for a long time the PR with the fix, until it was released on February |
@cxrod no worries! o/ |
This PR includes fixes for the following v4 issues:
VipsJpeg: Invalid SOS parameters for sequential JPEG
errors. This is observable with images taken with Samsung cameras, causing the api/cloudfront endpoint to output a blank json{}
instead of the image.Sample paths in which this was tested on aside from the unit tests:
/100x100/directory1/directory2/image.jpg
/fit-in/100x100/directory1/directory2/image.jpg
/fit-in/100x100/filters:someFilters/directory1/directory2/image.jpg
/filters:someFilters/directory1/directory2/image.jpg
/100x100/filters:someFilters/directory1/directory2/image_8x8x8x12.jpg
You may start using this PR/Fork with this Exported Function zip: https://github.com/innovationlove/serverless-image-handler/releases/tag/4.0.1-ilove
Just replace the code that your Serverless Image Handler Function is using with the package above. USE AT YOUR OWN RISK.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.