-
Notifications
You must be signed in to change notification settings - Fork 270
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
fix(ipx): fix retrieving images with query params #997
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #997 +/- ##
=======================================
Coverage 89.69% 89.69%
=======================================
Files 44 44
Lines 2920 2920
Branches 328 328
=======================================
Hits 2619 2619
Misses 300 300
Partials 1 1
☔ View full report in Codecov by Sentry. |
Thanks for this PR. (forwarding from discord) While this solution might work to solve encoding issues it seems not safer. Adding URLs to the ends of a URL is already not totally safe for parsers and UFO normalize util tends to avoid extra optional encodings (following the vue-router approach). By doing this change, we can potentially introduce new edge cases. We might investigate the upstream ipx server to handle double encoding. I will try to follow up on this soon. |
@pi0 Did you end up resolving upstream in |
Hi everyone @pi0 @danielroe any update regarding this? |
Hi @pi0 @danielroe 🙂, Can this PR be merged? It inherently fixes a key bug with nuxt generate when optimizing remote images with params. The params are getting double-encoded. Currently, the only workaround seems to be falling back to using the I recently noticed some heated debate on Reddit - https://www.reddit.com/r/Nuxt/comments/1e7t9dp/is_nuxt_fit_for_static_sites_or_astro/?utm_source=share&utm_medium=mweb3x&utm_name=mweb3xcss&utm_term=1&utm_content=share_button. The optimization to move this double-encoding avoidance upstream to ipx could be pursued separately. This probably fixes a few existing issues with nuxt generate. Unless I'm mistaken, please correct me if you see any regression with this normalization. Thanks 🙏🏾 bunch! |
Problem
images containing query params cannot be retrieved in IPX because the query params are encoded
issues
#815
#944
Solution
by using ufo's normalizeURL the src is split up in host, pathname, query etc before any encoding is done. This way only the path name is encoded
before
/_ipx/w_500%26%26q_80/https://host.com/uploads/image%20file.jpg%3Fv=1%26foo=bar
after
/_ipx/w_500%26%26q_80/https://host.com/uploads/image%20file.jpg?v=1&foo=bar
Remaining problem
This solves retrieving images, what remains to be fixed is writing the image to the disk as Nitro prevent saving images with routes containing "?" since 2.6.0:
nitrojs/nitro#1474