Repository navigation
Decode percent-encoded file paths in permalinks - #9097
Open
Ngo Quoc Viet (NgoQuocViet2001) wants to merge 1 commit into
Open
Ngo Quoc Viet (NgoQuocViet2001) wants to merge 1 commit into
Ngo Quoc Viet (NgoQuocViet2001) wants to merge 1 commit into
Conversation
Blob permalinks carry the file path percent-encoded, so a link to `docs/My%20Notes.md` was looked up as a file literally named `My%20Notes.md` and never opened locally. Decode the path before the existence check and the data-local-file attribute, and keep the raw path when it is not valid percent-encoding.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix preserves existing safety and fallback behavior, includes targeted regression tests, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Fixes local opening of blob permalinks in PR and issue overviews by decoding percent-encoded file paths before checking for local files.
Changes:
- Uses decoded paths for existence checks and
data-local-file, while preserving HTML escaping and the original href. - Keeps malformed percent-encoded paths unchanged.
- Adds regression tests for encoded spaces, brackets, quotes, and malformed encoding.
| File | Description |
|---|---|
src/test/common/utils.test.ts |
Adds decoding, HTML escaping, and fallback regression tests. |
src/common/utils.ts |
Decodes permalink paths before local lookup and attribute generation. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Blob permalinks in the PR and issue overviews are opened locally only when
fileExistsCheckfinds the file. ButprocessPermalinkstakes the path straight from the href, where it is still percent-encoded. A link todocs/My Notes.mdarrives asdocs/My%20Notes.md, the lookup goes to a file literally namedMy%20Notes.md, and the link stays a plain GitHub link. The same happens for non-ASCII file names and for paths that "Copy GitHub Permalink" encodes, such asapp/[slug]/page.tsx, which becomesapp/%5Bslug%5D/page.tsx.The path is now decoded before the existence check and before it is written to
data-local-file. It is still HTML-escaped after decoding, so%22ends up as". The href is unchanged. A path that is not valid percent-encoding, likesrc/100%.md, is used as written, as before.prComment.replacePermalinkandissueLinkLookup.findCodeLinkLocallyhave the same gap and are left for a separate change.Ran:
npm run test:preprocessandnode ./out/src/test/runTests.js: 1114 passing. The two new decoding tests fail without the fix.npm run test:scripts(75 passing) andnpm run test:webviews(96 passing)npm run compile,npm run lint,npm run hygieneandnpm run check:commands: clean