Encode querystring separators before URL turned into static path#2224
Encode querystring separators before URL turned into static path#2224TechAkayy wants to merge 1 commit into
Conversation
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2224 +/- ##
=======================================
Coverage 32.52% 32.52%
=======================================
Files 7 7
Lines 372 372
Branches 131 131
=======================================
Hits 121 121
Misses 194 194
Partials 57 57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/providers/ipxStatic.ts`:
- Line 17: Remove the manual pre-encoding step that replaces ?, &, = before
calling encodePath; specifically, stop constructing encodedSrc with
src.replace(...) and instead pass the original src into encodePath (i.e., call
encodePath(src)). If you still need to specially encode '=' for this provider,
perform that replacement only after encodePath has been applied. Update
references in this file to use the unmodified src with encodePath and remove the
pre-encoding logic around encodedSrc.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e02154d5-b0b0-470d-b204-b010dd1cb18f
📒 Files selected for processing (1)
src/runtime/providers/ipxStatic.ts
| } | ||
|
|
||
| const params = operationsGenerator(modifiers) || '_' | ||
| const encodedSrc = src.replace(/\?/g, '%3F').replace(/&/g, '%26').replace(/=/g, '%3D') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the relevant file to see lines 17 and 24 in context
sed -n '1,35p' src/runtime/providers/ipxStatic.tsRepository: nuxt/image
Length of output: 906
🏁 Script executed:
# Find where encodePath is imported from
rg "encodePath" src/runtime/providers/ipxStatic.ts -B 5 -A 2Repository: nuxt/image
Length of output: 378
🏁 Script executed:
# Check the ufo library documentation or source to understand encodePath behavior
rg "export.*encodePath|function encodePath" --type ts --type jsRepository: nuxt/image
Length of output: 36
🏁 Script executed:
# Check all usages of encodePath in the codebase
rg "encodePath\(" --type ts --type js -B 2 -A 1Repository: nuxt/image
Length of output: 2574
🏁 Script executed:
# Check if there's a package.json to see ufo version and potentially documentation
cat package.json | grep -A 2 '"ufo"'Repository: nuxt/image
Length of output: 102
🏁 Script executed:
# Look for any existing patterns of encoding/pre-encoding in other providers
rg "replace.*%[0-9A-F]" src/runtime/providers/ --type tsRepository: nuxt/image
Length of output: 561
🏁 Script executed:
# Check ufo library - look at its exported functions or any type definitions
npm list ufo 2>/dev/null || echo "npm not available, checking if there's documentation"
# Try to find any documentation or examples of encodePath behavior
rg "encodePath" . --type md --type txt 2>/dev/null || trueRepository: nuxt/image
Length of output: 17342
🏁 Script executed:
# Let's look at how glide.ts uses encodePath - it's a simpler case
cat -n src/runtime/providers/glide.ts | grep -A 5 -B 5 "encodePath"Repository: nuxt/image
Length of output: 731
🏁 Script executed:
# Compare ipxStatic with ipx to understand the difference
echo "=== IPX.TS (lines 75-85) ===" && sed -n '75,85p' src/runtime/providers/ipx.ts
echo ""
echo "=== IPXSTATIC.TS (lines 12-28) ===" && sed -n '12,28p' src/runtime/providers/ipxStatic.tsRepository: nuxt/image
Length of output: 699
🏁 Script executed:
# Let's look at the ufo library's encodePath implementation directly
find node_modules -name "*.d.ts" -o -name "*.ts" | xargs grep -l "encodePath" | grep ufo | head -1 | xargs cat | head -100Repository: nuxt/image
Length of output: 2758
🏁 Script executed:
# Search for ufo source code or type definitions
cat node_modules/.pnpm/ufo@1.6.3/node_modules/ufo/dist/index.d.ts 2>/dev/null | grep -A 5 "encodePath"Repository: nuxt/image
Length of output: 1387
🏁 Script executed:
# Let's test the actual behavior by creating a simple test
python3 << 'EOF'
# Standard URL encoding behavior
from urllib.parse import quote
src = "https://example.com/image?param=value&other=test"
# Pre-encoding approach (like ipxStatic does)
pre_encoded = src.replace("?", "%3F").replace("&", "%26").replace("=", "%3D")
print(f"Pre-encoded: {pre_encoded}")
# Then encoding again (what encodePath would do)
double_encoded = quote(pre_encoded, safe='/:')
print(f"After second encoding: {double_encoded}")
# Direct encoding approach (like ipx does)
direct = quote(src, safe='/:')
print(f"Direct encoding: {direct}")
print(f"\nDo they match? {double_encoded == direct}")
EOFRepository: nuxt/image
Length of output: 308
🏁 Script executed:
# Get the actual implementation of encodePath in ufo
cat node_modules/.pnpm/ufo@1.6.3/node_modules/ufo/dist/index.js | grep -A 10 "function encodePath"Repository: nuxt/image
Length of output: 146
🏁 Script executed:
# Let's also check the actual source code or README to understand the intent
cat node_modules/.pnpm/ufo@1.6.3/node_modules/ufo/README.md 2>/dev/null | grep -A 20 "encodePath" || echo "No README with encodePath info"Repository: nuxt/image
Length of output: 1433
Remove manual pre-encoding before encodePath to prevent double-encoding.
Line 17 pre-encodes ?, &, = into %3F, %26, %3D. When line 24 then calls encodePath(encodedSrc), the % character gets encoded again to %25, producing %253F, %2526, %253D. This breaks URL reconstruction.
The correct pattern is used in ipx.ts (line 80): call encodePath(src) directly without pre-encoding. If = must be encoded separately, do so only after the single encoding pass.
Proposed fix
- const encodedSrc = src.replace(/\?/g, '%3F').replace(/&/g, '%26').replace(/=/g, '%3D')
+ const encodedSrc = encodePath(src).replace(/=/g, '%3D')
if (!baseURL) {
baseURL = joinURL(ctx.options.nuxt.baseURL, '/_ipx')
}
return {
- url: joinURL(baseURL, params, encodePath(encodedSrc).replace(/\/{2,}/g, '/')),
+ url: joinURL(baseURL, params, encodedSrc.replace(/\/{2,}/g, '/')),
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/providers/ipxStatic.ts` at line 17, Remove the manual
pre-encoding step that replaces ?, &, = before calling encodePath; specifically,
stop constructing encodedSrc with src.replace(...) and instead pass the original
src into encodePath (i.e., call encodePath(src)). If you still need to specially
encode '=' for this provider, perform that replacement only after encodePath has
been applied. Update references in this file to use the unmodified src with
encodePath and remove the pre-encoding logic around encodedSrc.
2 participants
🔗 Linked issue
Resolves #2029 (comment)
Possibly #815 and #2039
📚 Description
When using ipxStatic (nuxt generate), if the remote image URL's own query string operators - ?, & and = are left raw, they can be interpreted as part of the IPX request URL/query instead of as part of the remote image URL. Encoding them keeps the full remote URL intact as the image source.
Reproduction -
nuxt generateandnpx serve .output/publicBefore fix (image doesn't load) - https://github.com/TechAkayy/nuxt-image-generate-issue/tree/main
After fix (image loads fine) - https://github.com/TechAkayy/nuxt-image-generate-issue/tree/pr-2224
Remote image URL like this:
https://images.unsplash.com/photo-1654870468927-92c943da24fe?crop=entropy&cs=srgb&fm=jpg&ixid=M3wyMDkyMnwwfDF8c2VhcmNofDQ5fHxiYWtlcnklMjBjYWtlfGVufDB8fHx8MTY5NTA5NzYyNXww&ixlib=rb-4.0.3&q=85gets transformed into:
https://images.unsplash.com/photo-1654870468927-92c943da24fe%3Fcrop%3Dentropy%26cs%3Dsrgb%26fm%3Djpg%26ixid%3DM3wyMDkyMnwwfDF8c2VhcmNofDQ5fHxiYWtlcnklMjBjYWtlfGVufDB8fHx8MTY5NTA5NzYyNXww%26ixlib%3Drb-4.0.3%26q%3D85before the URL is further transformed into ipx source path like this
/_ipx/w_10&q_70/https:/images.unsplash.com/...