-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add normalizePath and tests #6587
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ true | |
true | ||
true | ||
true | ||
|
||
''' | ||
""" | ||
# test os path creation, iteration, and deletion | ||
|
@@ -137,8 +138,69 @@ import times | |
let tm = fromUnix(0) + 100.microseconds | ||
writeFile("a", "") | ||
setLastModificationTime("a", tm) | ||
|
||
when defined(macosx): | ||
echo "true" | ||
else: | ||
echo getLastModificationTime("a") == tm | ||
removeFile("a") | ||
|
||
when defined(Linux) or defined(osx): | ||
|
||
block normalizedPath: | ||
block relative: | ||
doAssert normalizedPath(".") == "." | ||
doAssert normalizedPath("..") == ".." | ||
doAssert normalizedPath("../") == ".." | ||
doAssert normalizedPath("../..") == "../.." | ||
doAssert normalizedPath("../a/..") == ".." | ||
doAssert normalizedPath("../a/../") == ".." | ||
doAssert normalizedPath("./") == "." | ||
|
||
block absolute: | ||
doAssert normalizedPath("/") == "/" | ||
doAssert normalizedPath("/.") == "/" | ||
doAssert normalizedPath("/..") == "/" | ||
doAssert normalizedPath("/../") == "/" | ||
doAssert normalizedPath("/../..") == "/" | ||
doAssert normalizedPath("/../../") == "/" | ||
doAssert normalizedPath("/../../../") == "/" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it would make more sense to raise an error for these. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the point of normalization. If you are looking for a way to detect and prevent directory traversal I think that should go into joinPath There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @dom96 , throwing seems much saner behavior. EDIT likewise with python: even though i don't see that being documented... what's a use case where silently accepting is preferable to throwing? |
||
doAssert normalizedPath("/a/b/../../foo") == "/foo" | ||
doAssert normalizedPath("/a/b/../../../foo") == "/foo" | ||
doAssert normalizedPath("/./") == "/" | ||
doAssert normalizedPath("//") == "/" | ||
doAssert normalizedPath("///") == "/" | ||
doAssert normalizedPath("/a//b") == "/a/b" | ||
doAssert normalizedPath("/a///b") == "/a/b" | ||
doAssert normalizedPath("/a/b/c/..") == "/a/b" | ||
doAssert normalizedPath("/a/b/c/../") == "/a/b" | ||
|
||
else: | ||
|
||
block normalizedPath: | ||
block relative: | ||
doAssert normalizedPath(".") == "." | ||
doAssert normalizedPath("..") == ".." | ||
doAssert normalizedPath("..\\") == ".." | ||
doAssert normalizedPath("..\\..") == "..\\.." | ||
doAssert normalizedPath("..\\a\\..") == ".." | ||
doAssert normalizedPath("..\\a\\..\\") == ".." | ||
doAssert normalizedPath(".\\") == "." | ||
|
||
block absolute: | ||
doAssert normalizedPath("\\") == "\\" | ||
doAssert normalizedPath("\\.") == "\\" | ||
doAssert normalizedPath("\\..") == "\\" | ||
doAssert normalizedPath("\\..\\") == "\\" | ||
doAssert normalizedPath("\\..\\..") == "\\" | ||
doAssert normalizedPath("\\..\\..\\") == "\\" | ||
doAssert normalizedPath("\\..\\..\\..\\") == "\\" | ||
doAssert normalizedPath("\\a\\b\\..\\..\\foo") == "\\foo" | ||
doAssert normalizedPath("\\a\\b\\..\\..\\..\\foo") == "\\foo" | ||
doAssert normalizedPath("\\.\\") == "\\" | ||
doAssert normalizedPath("\\\\") == "\\" | ||
doAssert normalizedPath("\\\\\\") == "\\" | ||
doAssert normalizedPath("\\a\\\\b") == "\\a\\b" | ||
doAssert normalizedPath("\\a\\\\\\b") == "\\a\\b" | ||
doAssert normalizedPath("\\a\\b\\c\\..") == "\\a\\b" | ||
doAssert normalizedPath("\\a\\b\\c\\..\\") == "\\a\\b" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be in ospaths, no? nothing in
normalizePath
accesses file systemThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, I forgot about this. But @FedericoCeratto has waited long enough with this PR so we can fix this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of course; I much prefer faster PR turnaround than waiting forever for PR's to be so-called "perfect" (this slow turnaround really hurts D ecosystem IMO, Nim seems better in that regard)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=> kept track of that in #8177 meta-issue so we don't forget