Fix normalizePath rooting a relative path that begins with ./ followed by a slash#63587
Fix normalizePath rooting a relative path that begins with ./ followed by a slash#63587Osamaali313 wants to merge 1 commit into
Conversation
simpleNormalizePath stripped a leading './' before checking whether the following character was itself a separator. For input like './/a', the '/./' cleanup is a no-op, then startsWith('./') + slice(2) removed './' and left '/a' (the second slash was a redundant separator, not a root). '/a' then re-tested clean and was returned, so normalizePath('.//a') and getNormalizedAbsolutePath('.//a', '') produced the rooted '/a' instead of the relative 'a'. The whole './/...' and '././/...' family was affected.
Only strip the leading './' when it is an actual '.' segment, i.e. when the character after it is not another separator; otherwise fall through to the slow path, which normalizes correctly. Adds regression tests.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
1 similar comment
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
@Osamaali313 what coding agent are you using? |
|
I use Claude Code (an AI coding assistant). I review every change myself and verify each fix RED→GREEN against the real code before opening a PR — for this one I esbuild-bundled the compiler and ran an exhaustive differential over all strings up to length 8 from I've opened #63588 describing the bug, per the automation bot. Happy to adjust the fix or tests however you'd prefer. |
|
@Osamaali313 Any idea why Claude Code didn't read https://github.com/microsoft/TypeScript/blob/main/AGENTS.md#-do-not-create-coding-prs-for-this-repository ? |
|
You're right, and that's on me — I didn't read AGENTS.md before opening the PR; I went straight to the fix without checking this repo's agent policy, which is exactly the gap you're pointing at. Apologies for the noise. I understand TypeScript is in maintenance mode and a general bug fix like this isn't one of the accepted categories, so I won't open further PRs here and will direct anything like this to typescript-go instead. I've closed the linked issue (#63588) as well. |
Fixes #63588
Problem
simpleNormalizePath(the allocation-free fast path added in #60812) strips a leading./before checking whether the next character is itself a separator:For an input like
.//a, the/./cleanup is a no-op, thenstartsWith("./")is true andslice(2)removes./and leaves/a— but that second/was a redundant separator, not a root./athen re-tests clean and is returned, so a relative path is silently turned into a rooted one:.//a/aa.//a/b/a/ba/b././/a/aaThis affects both
normalizePathandgetNormalizedAbsolutePath. The slow-path component walker (the documented oracle) returns the correct relative result; only the fast path is wrong.Reproduction (real compiler, esbuild-bundled)
Fix
Only strip the leading
./when it is an actual.segment — i.e. when the character after it is not another separator; otherwise fall through to the slow path, which normalizes correctly:Verification
{'.', '/', 'a'}(9,840 inputs), original vs patched: 172 differing results, every one a./-followed-by-separator case where the patched output drops the spurious leading/to match the slow path; zero changes elsewhere.Test
Adds
.//a,.//a/b,././/aassertions to thegetNormalizedAbsolutePathunit test insrc/testRunner/unittests/paths.ts.