fix(ipx): encode ampersands in image uris

[danielroe]

🔗 Linked issue

closes https://github.com/nuxt/image/pull/1268

❓ Type of change

• [ ] 📖 Documentation (updates to the documentation or readme)
• [x] 🐞 Bug fix (a non-breaking change that fixes an issue)
• [ ] 👌 Enhancement (improving an existing functionality)
• [ ] ✨ New feature (a non-breaking change that adds functionality)
• [ ] 🧹 Chore (updates to the build process or auxiliary tools and libraries)
• [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

this encodes ampersands in image uris for ipx

remake of https://github.com/nuxt/image/pull/1268 by @riddla as I don't have permission to push to that branch

[cloudflare-workers-and-pages[bot]]

Deploying nuxt-image with    Cloudflare Pages

Latest commit:	178c5a0
Status:	✅  Deploy successful!
Preview URL:	https://d43e5445.nuxt-image.pages.dev
Branch Preview URL:	https://feat-encode-reserved-charact.nuxt-image.pages.dev

View logs

[pkg-pr-new[bot]]

Open in StackBlitz

npm i https://pkg.pr.new/@nuxt/image@2005

commit: 178c5a0

[codecov-commenter]

Codecov Report

:x: Patch coverage is 0% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 7.01%. Comparing base (356822d) to head (178c5a0).

Files with missing lines	Patch %	Lines
src/runtime/providers/ipx.ts	0.00%	1 Missing :warning:

Additional details and impacted files

@@           Coverage Diff            @@
##            main   #2005      +/-   ##
========================================
- Coverage   7.02%   7.01%   -0.01%     
========================================
  Files         77      77              
  Lines       3575    3576       +1     
  Branches     138     138              
========================================
  Hits         251     251              
- Misses      3276    3277       +1     
  Partials      48      48              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[riddla]

@danielroe, thank you for your work. I integrated your refactoring (locally) into my branch.

I think we might be encountering a bug in ipx – I pulled the latest ipx version locally and the 500 error seems to have to do with the order of the modifiers in the URL.

http://localhost:3001/embed%26f_webp%26s_400x400/picsum/id/237/600 does work, http://localhost:3001/q_20%26embed%26f_webp%26s_400x400/picsum/id/237/600 respectively using the quality modifier as the first modifier in the URL does trigger the 500 error (that you also encountered in the playground).

(The error is only triggered, when I use the encoded ampersands, http://localhost:3001/q_20&embed&f_webp&s_400x400/picsum/id/237/600 is fine.)

Where should we go from here? File a bug over at https://github.com/unjs/ipx?

[danielroe]

interesting. yes, in that case let's fix the issue in ipx

(or if it is as simple as sorting modifiers alphabetically we can do that in nuxt/image as a hotfix)

[danielroe]

@riddla I investigated and the issue is that the decoding happens after splitting by modifier, which must be either & or ,. (and , can cause issues within srcset.

we could either open a PR to ipx to decode the entire string first, or possibly we might choose a different separator? @pi0 may have some thoughts here.

[pi0]

if sorting can be a hotfix here would be great. in next major of IPX later we can change sep and make it configurable.

[riddla]

Just for reference: I re-tested the Slack behavior regarding ampersands this morning with https://www.buescher.de/_ipx/fit_inside&f_jpg&s_1200x630/img/preview/home.jpg respectively https://www.buescher.de/_ipx/fit_inside%26f_jpg%26s_1200x630/img/preview/home.jpg.

There is still no preview for URLs using ampersands: