imgix/imgix-rails

`ix_image_url` doesn't work with fingerprinted assets anymore

spaghetticode opened this issue · 3 comments

spaghetticode commented

Describe the bug

Starting from v4.3 the helper UrlHelper#ix_umage_url doesn't work with fingerprinted assets anymore, actually it breaks them.

The previous behavior was not optimal, as the resulting URL was including 2 question marks, still it was working at least for us.

The new behavior completely breaks the image URL which results in 404s.

I understand that fingerprinting assets with a timestamp is not best-practice anymore, but I believe this to be still pretty common especially in the Rails world with Paperclip.

To Reproduce

Imgix::Rails::VERSION
=> "4.2.0"
include Imgix::Rails::UrlHelper
=> Object
ix_image_url("some/image.jpg?123123", {some: "query"})
=> "https://example.imgix.net/some/image.jpg?123123?some=query" # <- - 2 question marks

Imgix::Rails::VERSION
=> "4.3.0"
include Imgix::Rails::UrlHelper
=> Object
ix_image_url("some/image.jpg?123123", {some: "query"})
=> "https://example.imgix.net/some/image.jpg%3F123123?some=query" # first question mark gets url-encoded, thus breaking the image URL

Expected behaviour

The first question mark is not URL-encoded anymore.

Information:

  • @imgix/imgix-rails version: v4.3.0

Additional context

I also opened an issue on imgix-rb since the breaking change was introduced there, but may be considered legit in that context.

luqven commented

Hey @spaghetticode, I just noticed this was the "parent" issue.

I can see why this is a breaking change for you. We should have more explicitly communicated that in the release notes and I'll make a note to add a test for URLs that include a ? in the path so that any future breaking changes there are caught before release.

Looks like to resolve your issues we'd need to expose a way to set disable_path_encoding globally in imgix-rails. I can't give you a time estimate as to when, but we'll look into adding this as a configuration option.

Of course we're always open to PRs and if you feel comfortable taking a stab we'd love to provide guidance and feedback.

In the meantime if anyone else would like to see this added please do comment and let us know to help us prioritize this work.

Thanks again for reporting this!

spaghetticode commented

@luqven hi! Thanks for the very quick response, it's very much appreciated 🙏 .

Yes, one easy and quick way for fixing this would be by setting disable_path_encoding, and I think we could benefit from that change. I'll be happy to provide a PR but currently I think I don't have the capacity for doing it, unfortunately I've been rather busy lately... but I'll try to find some time for it in the next days 🤞

  • ❤️1
luqven commented

Hey @spaghetticode 👋🏼

I just wanted to give you a big shoutout for all your hard work and for giving back to the community. To say thanks, we'd love to hook you up with some account credit. Please get in touch with support@imgix.com with your account info and send them a link to this comment. They'll get you set up.

We couldn't do it without people like you, and we are grateful for all you do to make imgix-rails the best it can be!

  • ❤️1