-
Notifications
You must be signed in to change notification settings - Fork 538
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
images in subfolders doesn't work #112
Comments
+1 We are experiencing the same issue. It should not assume |
Hey @raaone7 and @MartijnKooij: Thanks for your input on this, since this seems to be a popular feature request (working with subfolders using the Thumbor request format), I've gone ahead and added this to our backlog for a future release. In the meantime, try giving the new JSON-based request format a try, since you can specify subfolders within the "key" property similar to how an s3.getObject({}) request would work. This may provide a workaround for you guys in the meantime while we work on this. |
dear can you please fix this issue? |
Yes, please put this high on the priority list! |
@hayesry thanks for the acknowledgement. can you please advice us of an ETA when this issue would be fixed, and how would we update the existing setup with the fix you deliver. i suppose it would be delivered as a CFN update. |
In the meantime, try giving the new JSON-based request format a try, since you can specify subfolders within the "key" property similar to how an s3.getObject({}) request would work.
|
I am having similar issue, but strangely, I have one or two subfolder images that show, while the rest don't. Getting the error {"status":500,"code":"NoSuchKey","message":"The specified key does not exist."}. What is the solution for this? |
Same here, just found out it's an issue related to the path. Really need this feature, but going to use JSON request format for now. |
Can anyone please fix this issue i am not able to deploy this solution. |
Due to the fact that AWS have updated the lambda execution environment, which caused our old thumbor implementation to break, we had to implement this ourselves. If anyone is interested, this is the code we have so far (this is not tested in production yet, so no guarantees...but maybe helpful to someone) PLEASE NOTE: This was written in a way that works with the way that our application generates the Thumbor urls. It makes assumptions about the way the urls are constructed. It assumes that the dimensions are in the request somewhere, and does not handle all Thumbor options. It also assumes that anything it doesn't recognise after the image dimensions param is the start of the path, and then it takes the rest of the path from there. You will most likely not be able to use this without adapting it at least to some extent. /**
* Gets the S3 key of the file from the request path
* Please note: this function assumes that the dimensions will be in the request somewhere (eg /200x300/ or whatever)
* @param {Object} event - The request body.
*/
getFileS3Key(event) {
this.path = event.path;
const pathParts = this.path.split('/');
var fileS3Key = '', sizeParameterFound = false;
const matchSize = new RegExp(/^\d+x\d+$/);
for (var i = (securityHashPathIndex + 1); i < pathParts.length; i++) {
const pathPart = pathParts[i];
if (fileS3Key.length > 0) {
fileS3Key += '/';
}
else if (!sizeParameterFound && matchSize.test(pathPart)) {
sizeParameterFound = true;
continue;
}
else if (pathPart === ('fit-in') || pathPart.includes('filters:') || !sizeParameterFound) {
continue;
}
fileS3Key += pathPart;
}
return fileS3Key;
} securityHashPathIndex is 1 (in my case at least). and a unit test for that: // ----------------------------------------------------------------------------
// getFileS3Key()
// ----------------------------------------------------------------------------
describe('getFileS3Key()', function () {
describe('001/fileS3Key', function () {
it(`Should pass if the file S3 key is being extracted correctly from the url path`, function () {
// Arrange
const event = {
path: "/somesecurityhash/fit-in/200x300/filters:grayscale()/some/filexfile/path/blah/test-image-001.jpg"
}
// Act
const thumborMapping = new ThumborMapping();
var result = thumborMapping.getFileS3Key(event);
// Assert
const expectedResult = "some/filexfile/path/blah/test-image-001.jpg";
assert.deepEqual(result, expectedResult);
});
});
}); |
This solution is unusable for us without the ability to use subfolders. |
I'm in exactly the same boat as @soupy1976 here. I would happily move to Sharp from Thumbor, but Sharp won't layer images based on absolute remote urls. So here we are. My fix was:
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
) {
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.'
});
}
What the code essentially does it look for anything which isn't likely to be the start of a path, and once it no longer finds one it puts the remaining parts of the url into the returned key. Not heavily tested, but works with image paths in the form: |
@harrybailey v4.0 still uses sharpjs, even if you call it with the old Thumbor style request ... it simply translates that into a sharpjs request. So if you need functionality that Thumbor offers and Sharpjs does not, then you would need to use v3.0 still. I noticed someone has just put a fork of v3.0 which is fixed to work in the new Lambda environment though, so you could be in luck: jb |
You're spot on. It's not pulling in remote images. |
@harrybailey if you're thinking of making v4 work, then hopefully my post on stackoverflow will be helpful to you: jb |
this also caught us by surprise, the version 3 (python) just stopped working, so we had to get our hands with the version 4. anyway, here's one-liner fix that we did for our implementation, hope it'll be of help to anyone: -> #130 |
ATTENTION! We have a mobile app in production which most of its links broke all of sudden. We manage our S3 assets by organizing them in sub-folders, which was the root cause of this problem. Links like those stopped working: We tried a lot of solutions, we were about to do huge modifications to our infrastructure which will delay us like a week or so. However, so many thanks for @harrybailey who has saved us a lot of time with his ready-to-use code. For anyone who's searching for a solution for this issue, just follow @harrybailey's comment above, and you don't need to change anything in your current infrastructure. |
@harrybailey it works here! |
These edits seem to make my URLs work - but it doesn't seem to be doing the resize when I'm using an image in a subfolder, IE |
for anyone that using @harrybailey solution and had a problem with resizing, i attach my workaround on @rpong's PR, hope it helps |
We have updated our solution, and I believe your issue has been fixed. If you still see the issue with the latest version (v4.2), please feel free to reopen the issue. You can refer to the recent changes here |
you may try this yourself to reproduce
I tried this solution by putting test.jpg in root of the bucket - works perfect
eg. test123.com/test.jpg is the path location
test123.com/300x300/smart/diwali.jpg - works perfect
try the same image test.jpg in a subfolder of the same bucket - doesn't work, says keys not present
eg. test123.com/folder1/folder2/test.jpg is the path location
test123.com/300x300/smart/folder1/folder2/test.jpg - doesn't work
error in cloud watch
looks like while retrieving the key:
it should fetch based on
/smart/
context root instead of default/
. what do you think ?The text was updated successfully, but these errors were encountered: