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

[5.x] Support glide urls with URL params #11003

Open
wants to merge 6 commits into
base: 5.x
Choose a base branch
from

Conversation

ryanmitchell
Copy link
Contributor

This PR allows Glide to handle external urls with query params... eg: an instagram url like https://instagram.fbhd1-1.fna.fbcdn.net/v/t39.30808-6/459037616_18451217146036976_8749080258714687081_n.jpg?stp=dst-jpg_e35_s640x640_sh0.08&_nc_ht=instagram.fbhd1-1.fna.fbcdn.net&_nc_cat=108&_nc_ohc=4NMeg8elDJwQ7kNvgE1xSf8&_nc_gid=bb5d8bab5b84475797c0c5171eed8c1c&edm=APU89FAAAAAA&ccb=7-5&oh=00_AYCPWoEYVRiQPq2YmVzVEiYCmxC7rGDoJbR-r2o7DYePNg&oe=671F02D7&_nc_sid=bc0c2c

Closes: #7172

@jasonvarga
Copy link
Member

Haven't tested this, but...

Since the "path" is now the url without the query string, wouldn't this break if you had a URL structure where multiple images used the same url? Like http://site.com/image.php?name=foo.jpg and http://site.com/image.php?name=bar.jpg

@ryanmitchell
Copy link
Contributor Author

Yep thats fair, it wouldn't have worked. I've pushed this which should solve that case.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could add a test to ImageGeneratorTest that'd be great.

it_generates_an_image_by_external_url is already there so adding it_generates_an_image_by_external_url_with_query_string makes sense.

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

Successfully merging this pull request may close these issues.

Glide Tag: NotFoundHttpException when external src url contains query params
2 participants