Preserve leading .. for drive-relative paths during lexical normalization#302
Open
adityasingh2400 wants to merge 1 commit into
Open
Conversation
…zation
On Windows, a traditional drive-relative root such as `C:` anchors a path
to the current working directory on the named drive, not to a fixed
directory. A leading `..` is therefore meaningful: `C:..` is the parent of
the drive's current directory. `lexicallyNormalize()` was collapsing these
`..` components against the root as if `C:` were a fixed anchor, silently
changing the meaning of the path:
FilePath(#"C:..\foo\bar"#).lexicallyNormalized() // was C:foo\bar
FilePath(#"C:foo\..\..\bar"#).lexicallyNormalized() // was C:bar
Both forms now preserve the escaping `..` (C:..\foo\bar and C:..\bar
respectively), matching the behavior of ntpath.normpath. The fix keys off
whether the root anchors leading parents: only drive-relative `X:` roots
preserve a leading `..`. Rooted `\` and absolute roots (`C:\`, UNC, device)
still collapse it, so `\..\foo` continues to normalize to `\foo` and Unix
`/..` to `/`.
isLexicallyNormal is updated to agree, resolving the existing FIXME: a path
like `C:..\foo\bar` is now reported as lexically normal while `\..\foo\bar`
is not.
Adds regression coverage in FilePathSyntaxTest for leading and interior
`..` against both drive-relative and rooted Windows paths.
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.
On Windows, a traditional drive-relative root like
C:anchors a path to the current working directory on the named drive, not to a fixed location. That makes a leading..meaningful:C:..is the parent of the drive's current directory, which is generally not the root of the drive.lexicallyNormalize()was treating any root as a fixed anchor and collapsing leading..against it, so it silently changed the meaning of drive-relative paths:In each case a
..that escapes the relative portion was dropped, which loses information.The root cause is in
_normalizeSpecialDirectories: the branch that skips a leading..fired whenever the path had a root (hasRoot && writeIdx == relStart). That is correct for fixed anchors (/on Unix, and\,C:\, UNC and device roots on Windows, where..at the front refers to the parent of the root and collapses to the root), but it is wrong for the drive-relativeC:form, where the..has to be kept just like a leading..on a rootless relative path.The fix distinguishes the two cases. A new internal
Root._isTraditionalDriveRelativeidentifies theX:form (a two-character root ending in:, which the existingisAbsolutelogic already treats as the only colon-terminated relative root). Normalization now preserves a leading..exactly when the root, if any, is drive-relative.isLexicallyNormalis updated to match, which resolves the FIXME that was sitting on it:C:..\foo\baris now reported as lexically normal, while\..\foo\baris not.After the change:
These match
ntpath.normpathfor the same inputs (CPython recently corrected its own drive-relative normalization for the same reason in python/cpython#126780). The rooted\and absolute cases, and all Unix behavior such as/.. => /, are unaffected.Verification: reproduced the wrong output against the documented/reference behavior using the package's
withWindowsPathstest harness on macOS, applied the fix, and confirmed the outputs match. Added regression cases inFilePathSyntaxTestcovering leading and interior..for both drive-relative (C:) and rooted (\) Windows paths; they fail before the change and pass after.swift testpasses in both debug and release (67 XCTest tests and 7 swift-testing tests, 0 failures), and one pre-existing case that encoded the old behavior (C:foo\bar\..\..\.. => C:) was corrected toC:...This is related to the drive-root discussion in #188 (the trapping/validation questions there are separate and not addressed here).
I have not yet completed the Swift project CLA; happy to sign it as part of the review process per CONTRIBUTING.