diff options
author | havardjohn <havard.mjaavatten@outlook.com> | 2022-09-04 02:47:09 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-03 20:47:09 -0400 |
commit | 23e0160af283bb0bb573a86145e6c1c792780d49 (patch) | |
tree | d95dbd0c940f44d3265af57073f78ba1bb6934dd /tests | |
parent | a6189fbb988ae9c9e6760cb901e792e043b9086b (diff) | |
download | Nim-23e0160af283bb0bb573a86145e6c1c792780d49.tar.gz |
Add improved Windows UNC path support in std/os (#20281)
* Add improved Windows UNC path support in std/os Original issue: `std/os.createDir` tries to create every component of the given path as a directory. The problem is that `createDir` interprets every backslash/slash as a path separator. For a UNC path this is incorrect. E.g. one UNC form is `\\Server\Volume\Path`. It's an error to create the `\\Server` directory, as well as creating `\\Server\Volume`. Add `ntpath.nim` module with `splitDrive` proc. This implements UNC path parsing as implemented in the Python `ntpath.py` module. The following UNC forms are supported: * `\\Server\Volume\Path` * `\\?\Volume\Path` * `\\?\UNC\Server\Volume\Path` Improves support for UNC paths in various procs in `std/os`: --- * pathnorm.addNormalizePath * Issue: This had incomplete support for UNC paths * The UNC prefix (first 2 characters of a UNC path) was assumed to be exactly `\\`, but it can be `//` and `\/`, etc. as well * Also, the UNC prefix must be normalized to the `dirSep` argument of `addNormalizePath` * Resolution: Changed to account for different UNC prefixes, and normalizing the prefixes according to `dirSep` * Affected procs that get tests: `relativePath`, `joinPath` * Issue: The server/volume part of UNC paths can be stripped when normalizing `..` path components * This error should be negligable, so ignoring this * splitPath * Now make sure the UNC drive is not split; return the UNC drive as `head` if the UNC drive is the only component of the path * Consequently fixes `extractFilename`, `lastPathPart` * parentDir / `/../` * Strip away drive before working on the path, prepending the drive after all work is done - prevents stripping UNC components * Return empty string if drive component is the only component; this is the behavior for POSIX paths as well * Alternative implementation: Just call something like `pathnorm.normalizePath(path & "/..")` for the whole proc - maybe too big of a change * tailDir * If drive is present in path, just split that from path and return path * parentDirs iterator * Uses `parentDir` for going backwards * When going forwards, first `splitDrive`, yield the drive field, and then iterate over path field as normal * splitFile * Make sure path parsing stops at end of drive component * createDir * Fixed by skipping drive part before creating directories * Alternative implementation: use `parentDirs` iterator instead of iterating over characters * Consequence is that it will try to create the root directory * isRootDir * Changed to treat UNC drive alone as root (e.g. "//?/c:" is root) * This change prevents the empty string being yielded by the `parentDirs` iterator with `fromRoot = false` * Internal `sameRoot` * The "root" refers to the drive, so `splitDrive` can be used here This adds UNC path support to all procs that could use it in std/os. I don't think any more work has to be done to support UNC paths. For the future, I believe the path handling code can be refactored due to duplicate code. There are multiple ways of manipulating paths, such as manually searching string for path separator and also having a path normalizer (pathnorm.nim). If all path manipulation used `pathnorm.nim`, and path component splitting used `parentDirs` iterator, then a lot of code could be removed. Tests --- Added test file for `pathnorm.nim` and `ntpath.nim`. `pathnorm.normalizePath` has no tests, so I'm adding a few unit tests. `ntpath.nim` contains tests copied from Python's test suite. Added integration tests to `tos.nim` that tests UNC paths. Removed incorrect `relativePath` runnableExamples from being tested on Windows: --- `relativePath("/Users///me/bar//z.nim", "//Users/", '/') == "me/bar/z.nim"` This is incorrect on Windows because the `/` and `//` are not the same root. `/` (or `\`) is expanded to the drive in the current working directory (e.g. `C:\`). `//` (or `\\`), however, are the first two characters of a UNC path. The following holds true for normal Windows installations: * `dirExists("/Users") != dirExists("//Users")` * `dirExists("\\Users") != dirExists("\\\\Users")` Fixes #19103 Questions: --- * Should the `splitDrive` proc be in `os.nim` instead with copyright notice above the proc? * Is it fine to put most of the new tests into the `runnableExamples` section of the procs in std/os? * [skipci] Apply suggestions from code review Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com> * [skip ci] Update lib/pure/os.nim Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com> * Move runnableExamples tests in os.nim to tos.nim * tests/topt_no_cursor: Change from using splitFile to splitDrive `splitFile` can no longer be used in the test, because it generates different ARC code on Windows and Linux. This replaces `splitFile` with `splitDrive`, because it generates same ARC code on Windows and Linux, and returns a tuple. I assume the test wants a proc that returns a tuple. * Drop copyright attribute to Python Co-authored-by: Clay Sweetser <Varriount@users.noreply.github.com>
Diffstat (limited to 'tests')
-rw-r--r-- | tests/arc/topt_no_cursor.nim | 35 | ||||
-rw-r--r-- | tests/stdlib/tntpath.nim | 48 | ||||
-rw-r--r-- | tests/stdlib/tos.nim | 70 | ||||
-rw-r--r-- | tests/stdlib/tpathnorm.nim | 34 |
4 files changed, 165 insertions, 22 deletions
diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim index f9c789eb9..9585c04a0 100644 --- a/tests/arc/topt_no_cursor.nim +++ b/tests/arc/topt_no_cursor.nim @@ -1,5 +1,5 @@ discard """ - output: '''(repo: "", package: "meo", ext: "") + output: '''(package: "", ext: "meo") doing shady stuff... 3 6 @@ -11,24 +11,17 @@ doing shady stuff... cmd: '''nim c --gc:arc --expandArc:newTarget --expandArc:delete --expandArc:p1 --expandArc:tt --hint:Performance:off --assertions:off --expandArc:extractConfig --expandArc:mergeShadowScope --expandArc:check $file''' nimout: '''--expandArc: newTarget -var - splat - :tmp - :tmp_1 - :tmp_2 -splat = splitFile(path) -:tmp = splat.dir -wasMoved(splat.dir) -:tmp_1 = splat.name -wasMoved(splat.name) -:tmp_2 = splat.ext -wasMoved(splat.ext) +splat = splitDrive do: + let blitTmp = path + blitTmp +:tmp = splat.drive +wasMoved(splat.drive) +:tmp_1 = splat.path_1 +wasMoved(splat.path_1) result = ( - let blitTmp = :tmp - blitTmp, - let blitTmp_1 = :tmp_1 + let blitTmp_1 = :tmp blitTmp_1, - let blitTmp_2 = :tmp_2 + let blitTmp_2 = :tmp_1 blitTmp_2) `=destroy`(splat) -- end of expandArc ------------------------ @@ -160,13 +153,13 @@ else: -- end of expandArc ------------------------''' """ -import os +import os, std/private/ntpath -type Target = tuple[repo, package, ext: string] +type Target = tuple[package, ext: string] proc newTarget*(path: string): Target = - let splat = path.splitFile - result = (repo: splat.dir, package: splat.name, ext: splat.ext) + let splat = path.splitDrive + result = (package: splat.drive, ext: splat.path) echo newTarget("meo") diff --git a/tests/stdlib/tntpath.nim b/tests/stdlib/tntpath.nim new file mode 100644 index 000000000..39798d816 --- /dev/null +++ b/tests/stdlib/tntpath.nim @@ -0,0 +1,48 @@ +discard """ +""" + +import std/private/ntpath + +block: # From Python's `Lib/test/test_ntpath.py` + doAssert splitDrive(r"c:\foo\bar") == (r"c:", r"\foo\bar") + doAssert splitDrive(r"c:/foo/bar") == (r"c:", r"/foo/bar") + doAssert splitDrive(r"\\conky\mountpoint\foo\bar") == (r"\\conky\mountpoint", r"\foo\bar") + doAssert splitDrive(r"//conky/mountpoint/foo/bar") == (r"//conky/mountpoint", r"/foo/bar") + doAssert splitDrive(r"\\\conky\mountpoint\foo\bar") == (r"", r"\\\conky\mountpoint\foo\bar") + doAssert splitDrive(r"///conky/mountpoint/foo/bar") == (r"", r"///conky/mountpoint/foo/bar") + doAssert splitDrive(r"\\conky\\mountpoint\foo\bar") == (r"", r"\\conky\\mountpoint\foo\bar") + doAssert splitDrive(r"//conky//mountpoint/foo/bar") == (r"", r"//conky//mountpoint/foo/bar") + # Issue #19911: UNC part containing U+0130 + doAssert splitDrive(r"//conky/MOUNTPOİNT/foo/bar") == (r"//conky/MOUNTPOİNT", r"/foo/bar") + # gh-81790: support device namespace, including UNC drives. + doAssert splitDrive(r"//?/c:") == (r"//?/c:", r"") + doAssert splitDrive(r"//?/c:/") == (r"//?/c:", r"/") + doAssert splitDrive(r"//?/c:/dir") == (r"//?/c:", r"/dir") + doAssert splitDrive(r"//?/UNC") == (r"", r"//?/UNC") + doAssert splitDrive(r"//?/UNC/") == (r"", r"//?/UNC/") + doAssert splitDrive(r"//?/UNC/server/") == (r"//?/UNC/server/", r"") + doAssert splitDrive(r"//?/UNC/server/share") == (r"//?/UNC/server/share", r"") + doAssert splitDrive(r"//?/UNC/server/share/dir") == (r"//?/UNC/server/share", r"/dir") + doAssert splitDrive(r"//?/VOLUME{00000000-0000-0000-0000-000000000000}/spam") == (r"//?/VOLUME{00000000-0000-0000-0000-000000000000}", r"/spam") + doAssert splitDrive(r"//?/BootPartition/") == (r"//?/BootPartition", r"/") + + doAssert splitDrive(r"\\?\c:") == (r"\\?\c:", r"") + doAssert splitDrive(r"\\?\c:\") == (r"\\?\c:", r"\") + doAssert splitDrive(r"\\?\c:\dir") == (r"\\?\c:", r"\dir") + doAssert splitDrive(r"\\?\UNC") == (r"", r"\\?\UNC") + doAssert splitDrive(r"\\?\UNC\") == (r"", r"\\?\UNC\") + doAssert splitDrive(r"\\?\UNC\server\") == (r"\\?\UNC\server\", r"") + doAssert splitDrive(r"\\?\UNC\server\share") == (r"\\?\UNC\server\share", r"") + doAssert splitDrive(r"\\?\UNC\server\share\dir") == (r"\\?\UNC\server\share", r"\dir") + doAssert splitDrive(r"\\?\VOLUME{00000000-0000-0000-0000-000000000000}\spam") == (r"\\?\VOLUME{00000000-0000-0000-0000-000000000000}", r"\spam") + doAssert splitDrive(r"\\?\BootPartition\") == (r"\\?\BootPartition", r"\") + +block: + doAssert splitDrive(r"C:") == (r"C:", r"") + doAssert splitDrive(r"C:\") == (r"C:", r"\") + doAssert splitDrive(r"non/absolute/path") == (r"", r"non/absolute/path") + + # Special for `\`-rooted paths on Windows. I don't know if this is correct, + # rbut `\` is not recognized as a drive, in contrast to `C:` or `\?\c:`. + # This behavior is the same for Python's `splitdrive` function. + doAssert splitDrive(r"\\") == (r"", r"\\") diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim index b7816fd41..08088d707 100644 --- a/tests/stdlib/tos.nim +++ b/tests/stdlib/tos.nim @@ -508,7 +508,11 @@ block ospaths: doAssert relativePath("/Users/me/bar/z.nim", "/Users/other/bad", '/') == "../../me/bar/z.nim" doAssert relativePath("/Users/me/bar/z.nim", "/Users/other", '/') == "../me/bar/z.nim" - doAssert relativePath("/Users///me/bar//z.nim", "//Users/", '/') == "me/bar/z.nim" + + # `//` is a UNC path, `/` is the current working directory's drive, so can't + # run this test on Windows. + when not doslikeFileSystem: + doAssert relativePath("/Users///me/bar//z.nim", "//Users/", '/') == "me/bar/z.nim" doAssert relativePath("/Users/me/bar/z.nim", "/Users/me", '/') == "bar/z.nim" doAssert relativePath("", "/users/moo", '/') == "" doAssert relativePath("foo", "", '/') == "foo" @@ -707,3 +711,67 @@ block: # isAdmin if isAzure and defined(windows): doAssert isAdmin() # In Azure on POSIX tests run as a normal user if isAzure and defined(posix): doAssert not isAdmin() + +when doslikeFileSystem: + import std/[sequtils, private/ntpath] + + block: # Bug #19103 UNC paths + + # Easiest way of generating a valid, readable and writable UNC path + let tempDir = r"\\?\" & getTempDir() + doAssert dirExists tempDir + createDir tempDir / "test" + removeDir tempDir / "test" + createDir tempDir / "recursive" / "test" + removeDir tempDir / "recursive" / "test" + + let tempDir2 = getTempDir() + let (drive, pathNoDrive) = splitDrive(tempDir2) + setCurrentDir drive + doAssert cmpIgnoreCase(getCurrentDir().splitDrive.drive, drive) == 0 + + # Test `\Users` path syntax on Windows by stripping away drive. `\` + # resolves to the drive in current working directory. This drive will be + # the same as `tempDir2` because of the `setCurrentDir` above. + doAssert pathNoDrive[0] == '\\' + createDir pathNoDrive / "test" + doAssert dirExists pathNoDrive / "test" + removeDir pathNoDrive / "test" + + doAssert splitPath("//?/c:") == ("//?/c:", "") + + doAssert relativePath("//?/c:///Users//me", "//?/c:", '/') == "Users/me" + + doAssert parentDir(r"\\?\c:") == r"" + doAssert parentDir(r"//?/c:/Users") == r"\\?\c:" + doAssert parentDir(r"\\localhost\c$") == r"" + doAssert parentDir(r"\Users") == r"\" + + doAssert tailDir("//?/c:") == "" + doAssert tailDir("//?/c:/Users") == "Users" + doAssert tailDir(r"\\localhost\c$\Windows\System32") == r"Windows\System32" + + doAssert isRootDir("//?/c:") + doAssert isRootDir("//?/UNC/localhost/c$") + doAssert not isRootDir(r"\\?\c:\Users") + + doAssert parentDirs(r"C:\Users", fromRoot = true).toSeq == @[r"C:\", r"C:\Users"] + doAssert parentDirs(r"C:\Users", fromRoot = false).toSeq == @[r"C:\Users", r"C:"] + doAssert parentDirs(r"\\?\c:\Users", fromRoot = true).toSeq == + @[r"\\?\c:\", r"\\?\c:\Users"] + doAssert parentDirs(r"\\?\c:\Users", fromRoot = false).toSeq == + @[r"\\?\c:\Users", r"\\?\c:"] + doAssert parentDirs(r"//localhost/c$/Users", fromRoot = true).toSeq == + @[r"//localhost/c$/", r"//localhost/c$/Users"] + doAssert parentDirs(r"//?/UNC/localhost/c$/Users", fromRoot = false).toSeq == + @[r"//?/UNC/localhost/c$/Users", r"\\?\UNC\localhost\c$"] + doAssert parentDirs(r"\Users", fromRoot = true).toSeq == @[r"\", r"\Users"] + doAssert parentDirs(r"\Users", fromRoot = false).toSeq == @[r"\Users", r"\"] + + doAssert r"//?/c:" /../ "d/e" == r"\\?\c:\d\e" + doAssert r"//?/c:/Users" /../ "d/e" == r"\\?\c:\d\e" + doAssert r"\\localhost\c$" /../ "d/e" == r"\\localhost\c$\d\e" + + doAssert splitFile("//?/c:") == ("//?/c:", "", "") + doAssert splitFile("//?/c:/Users") == ("//?/c:", "Users", "") + doAssert splitFile(r"\\localhost\c$\test.txt") == (r"\\localhost\c$", "test", ".txt") diff --git a/tests/stdlib/tpathnorm.nim b/tests/stdlib/tpathnorm.nim new file mode 100644 index 000000000..2cb644e3c --- /dev/null +++ b/tests/stdlib/tpathnorm.nim @@ -0,0 +1,34 @@ +discard """ +""" + +import std/os + +when doslikeFileSystem: + import std/pathnorm + + template initVars = + var state {.inject.} = 0 + var result {.inject.}: string + + block: # / -> / + initVars + addNormalizePath("//?/c:/./foo//bar/../baz", result, state, '/') + doAssert result == "//?/c:/foo/baz" + addNormalizePath("me", result, state, '/') + doAssert result == "//?/c:/foo/baz/me" + + block: # / -> \ + initVars + addNormalizePath(r"//?/c:/./foo//bar/../baz", result, state, '\\') + doAssert result == r"\\?\c:\foo\baz" + addNormalizePath("me", result, state, '\\') + doAssert result == r"\\?\c:\foo\baz\me" + + block: # Append path component to UNC drive + initVars + addNormalizePath(r"//?/c:", result, state, '\\') + doAssert result == r"\\?\c:" + addNormalizePath("Users", result, state, '\\') + doAssert result == r"\\?\c:\Users" + addNormalizePath("me", result, state, '\\') + doAssert result == r"\\?\c:\Users\me" |