Skip to content

Don't re-encode the file parameter in the viewer - #21718

Merged
timvandermeij merged 1 commit into
mozilla:masterfrom
calixteman:fix/file-param-encoding
Aug 6, 2026
Merged

Don't re-encode the file parameter in the viewer#21718
timvandermeij merged 1 commit into
mozilla:masterfrom
calixteman:fix/file-param-encoding

Conversation

@calixteman

Copy link
Copy Markdown
Contributor

parseQueryString, i.e. URLSearchParams, has already percent-decoded the parameter, hence re-encoding it with encodeURIComponent and only restoring the slashes leaves e.g. "?", "&" and "%" escaped. This breaks relative URLs with a query string, e.g. ?file=%2Fget.jsp%3Fid%3D1%26x%3D2, and relative URLs with a percent-encoded path.

The value is now used as-is, except for a "#" in a relative URL which is still escaped: since the viewer takes its own hash parameters from the viewer URL, a "#" in the file parameter is assumed to be part of the filename (see #19990).

It fixes #20137.

`parseQueryString`, i.e. `URLSearchParams`, has already percent-decoded the
parameter, hence re-encoding it with `encodeURIComponent` and only restoring
the slashes leaves e.g. "?", "&" and "%" escaped. This breaks relative URLs
with a query string, e.g. `?file=%2Fget.jsp%3Fid%3D1%26x%3D2`, and relative
URLs with a percent-encoded path.

The value is now used as-is, except for a "#" in a relative URL which is
still escaped: since the viewer takes its own hash parameters from the
*viewer* URL, a "#" in the `file` parameter is assumed to be part of the
filename (see mozilla#19990).

It fixes mozilla#20137.
@codecov-commenter

codecov-commenter commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (72a76e5) to head (0ac3b88).
⚠️ Report is 26 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #21718      +/-   ##
==========================================
+ Coverage   90.03%   90.05%   +0.01%     
==========================================
  Files         264      264              
  Lines       66894    66937      +43     
==========================================
+ Hits        60229    60280      +51     
+ Misses       6665     6657       -8     
Flag Coverage Δ
browsertest 66.48% <ø> (-0.04%) ⬇️
integrationtest 69.36% <100.00%> (-0.01%) ⬇️
unittest 57.98% <ø> (+0.09%) ⬆️
unittestcli 56.70% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Snuffleupagus

Copy link
Copy Markdown
Collaborator

Does this perhaps fix issue #20218 as well?

@calixteman

Copy link
Copy Markdown
Contributor Author

Does this perhaps fix issue #20218 as well?

Yes it does.

@timvandermeij
timvandermeij merged commit 93c2ae4 into mozilla:master Aug 6, 2026
17 checks passed
@timvandermeij

Copy link
Copy Markdown
Collaborator

Thank you for fixing this and extending the test coverage!

@calixteman
calixteman deleted the fix/file-param-encoding branch August 6, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants