Skip to content
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

Thumbor paths broken if they include "-" and "100x100" #208

Closed
Furytron opened this issue Apr 15, 2020 · 12 comments
Closed

Thumbor paths broken if they include "-" and "100x100" #208

Furytron opened this issue Apr 15, 2020 · 12 comments

Comments

@Furytron
Copy link

Furytron commented Apr 15, 2020

Hi All,

Hope someone can help me with this one. I've got v4.2 deployed and we have stumbled onto an odd issue. I've looked at the open and closed tickets and could only see 163 which might be related but is not the same.

When an S3 key has a hypen "-" and is then followed by two numbers that are separated by (lowercase) "x" either in the path or the filename the image is not returned to you when you go to the cloudfront URL.

Some examples of broken URLs that only return an empty object: {} with a 200 response.

To narrow down the scope of the problem, I tried the following URLs with altered but similar filenames and they all work correctly. Images are returned.

The same is happening for paths in the S3 key, but you get a different response (404) from the browser. For example a broken URL with the values in the path:

This time you get the following returned:
status 404 code "NoSuchKey" message "The specified key does not exist."

I think the issue is being caused by the confusion with the "fit-in" filter: /fit-in/800x1000/ but that is just a guess.

For the path issue, it might be related to the code in image-request.js.

A code test using that function shows that the URL paths are broken: https://jsfiddle.net/Furytron/bqjf07zu/2/ when they include those characters, in that order. But this code does not break for the filename. So there may be two issues occurring here.

Any help would be appreciated on this.

@shortdoormedia
Copy link

shortdoormedia commented Apr 19, 2020

I am not sure if it is the exact same issue but i was having trouble with file names that included any dimensions. See my post here: #176 (comment)

I had to add a lambda function in order to convert the uri requests into something that was consumable.

Screen Shot 2020-04-19 at 1 48 13 PM

@Furytron
Copy link
Author

Thanks @shortdoormedia for the reply and link to your issue. I knew there had to have been an issue logged like this. I just missed it.

So it appears from that thread that the issue was only partially fixed.

I'm not 100% sure I understand that comment you have linked from @beomseoklee, he appears to say that its not supported having files with those names.

In my case, the image in the S3 key: https://test.cloudfront.net/test/test-100x100.jpg would have already been a resized image from our application and the "-100x100" automatically gets appended to the name to differentiate it from the original.

I will have a look at your solution for the lambda, thanks again for posting!

@shortdoormedia
Copy link

shortdoormedia commented Apr 20, 2020

In your case it's trying to grab an image dynamically sized at: -100x100. The function/bug only grabs the first instance of dimensions from a uri request string.

You need to re-write your request to:
https://test.cloudfront.net/fit-in/100x100/test/test-100x100.jpg

@Furytron
Copy link
Author

Furytron commented Apr 20, 2020

@shortdoormedia

Just to note that I am not actually applying any filters when I request
https://test.cloudfront.net/test/test-100x100.jpg. I haven't even applied filters yet. The "-100x100" is just in the name of the file, I don't want anything to be resized or changed. So the URL doesn't return the image since it has that string in the name.

@beomseoklee
Copy link
Member

@Furytron I'm sorry to hear that you experienced an issue with Serverless Image Handler. I have reproduced this one, so we will be back soon for this issue.

@Furytron
Copy link
Author

Thanks @beomseoklee !

@shortdoormedia
Copy link

@Furytron yeah, it doesn't matter if you are trying to resize, if your file name has dimensions in it, it thinks you are trying to resize. That is the issue.

@BuiltByJames
Copy link

Also getting this issue. Any update @beomseoklee ?

@beomseoklee
Copy link
Member

beomseoklee commented Aug 25, 2020

I haven't tested whole cases, but simply, we can change the regular expression to this:

// thumbor-mapping.js
const dimPath = this.path.match(/\/((\d+x\d+)|(0x\d+))\//g);
if (dimPath) {
    const dims = dimPath[0].replace(/\//g, '').split('x');

I'll be back when I'm done the full testing.

@MaartenTutak
Copy link

Any updates? Running into the same issue here.

@beomseoklee
Copy link
Member

Thanks for waiting. This one is fixed in v5.1.0.

@Furytron
Copy link
Author

Thanks @beomseoklee !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants