Skip to content

Commit

Permalink
Fix isInvalidFilename & always operate on the full passed in string (#…
Browse files Browse the repository at this point in the history 
…19004)

* Fix isInvalidFilename segfault & deprecate the method

* Update lib/pure/os.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

* Update lib/pure/os.nim

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
  • Loading branch information
@johnnovak @konsumlamm
johnnovak and konsumlamm authored Oct 20, 2021
1 parent c239db4 commit 62701cd
Showing 1 changed file with 46 additions and 45 deletions.
91 changes: 46 additions & 45 deletions lib/pure/os.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ const weirdTarget = defined(nimscript) or defined(js)
since (1, 1):
const
invalidFilenameChars* = {'/', '\\', ':', '*', '?', '"', '<', '>', '|', '^', '\0'} ## \
## Characters that may produce invalid filenames across Linux, Windows, Mac, etc.
## You can check if your filename contains these char and strip them for safety.
## Characters that may produce invalid filenames across Linux, Windows and Mac.
## You can check if your filename contains any of these chars and strip them for safety.
## Mac bans ``':'``, Linux bans ``'/'``, Windows bans all others.
invalidFilenames* = [
"CON", "PRN", "AUX", "NUL",
Expand Down Expand Up @@ -297,7 +297,7 @@ proc isAbsolute*(path: string): bool {.rtl, noSideEffect, extern: "nos$1", raise
result = path[0] == '$'
elif defined(posix) or defined(js):
# `or defined(js)` wouldn't be needed pending https://github.com/nim-lang/Nim/issues/13469
# This works around the problem for posix, but windows is still broken with nim js -d:nodejs
# This works around the problem for posix, but Windows is still broken with nim js -d:nodejs
result = path[0] == '/'
else:
doAssert false # if ever hits here, adapt as needed
Expand All @@ -321,7 +321,7 @@ when doslikeFileSystem:
proc sameRoot(path1, path2: string): bool {.noSideEffect, raises: [].} =
## Return true if path1 and path2 have a same root.
##
## Detail of windows path formats:
## Detail of Windows path formats:
## https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats

assert(isAbsolute(path1))
Expand Down Expand Up @@ -370,7 +370,7 @@ proc relativePath*(path, base: string, sep = DirSep): string {.
## this can be useful to ensure the relative path only contains `'/'`
## so that it can be used for URL constructions.
##
## On windows, if a root of `path` and a root of `base` are different,
## On Windows, if a root of `path` and a root of `base` are different,
## returns `path` as is because it is impossible to make a relative path.
## That means an absolute path can be returned.
##
Expand Down Expand Up @@ -558,27 +558,25 @@ iterator parentDirs*(path: string, fromRoot=false, inclusive=true): string =
## See also:
## * `parentDir proc <#parentDir,string>`_
##
## **Examples:**
##
## .. code-block::
## let g = "a/b/c"
##
## for p in g.parentDirs:
## echo p
## # a/b/c
## # a/b
## # a
##
## for p in g.parentDirs(fromRoot=true):
## echo p
## # a/
## # a/b/
## # a/b/c
##
## for p in g.parentDirs(inclusive=false):
## echo p
## # a/b
## # a
runnableExamples:
let g = "a/b/c"

for p in g.parentDirs:
echo p
# a/b/c
# a/b
# a

for p in g.parentDirs(fromRoot=true):
echo p
# a/
# a/b/
# a/b/c

for p in g.parentDirs(inclusive=false):
echo p
# a/b
# a

if not fromRoot:
var current = path
Expand Down Expand Up @@ -944,8 +942,7 @@ proc getCacheDir*(): string =
proc getCacheDir*(app: string): string =
## Returns the cache directory for an application `app`.
##
## * On windows, this uses: `getCacheDir() / app / "cache"`
##
## * On Windows, this uses: `getCacheDir() / app / "cache"`
## * On other platforms, this uses: `getCacheDir() / app`
when defined(windows):
getCacheDir() / app / "cache"
Expand Down Expand Up @@ -1988,7 +1985,7 @@ proc removeFile*(file: string) {.rtl, extern: "nos$1", tags: [WriteDirEffect], n
proc tryMoveFSObject(source, dest: string, isDir: bool): bool {.noWeirdTarget.} =
## Moves a file (or directory if `isDir` is true) from `source` to `dest`.
##
## Returns false in case of `EXDEV` error or `AccessDeniedError` on windows (if `isDir` is true).
## Returns false in case of `EXDEV` error or `AccessDeniedError` on Windows (if `isDir` is true).
## In case of other errors `OSError` is raised.
## Returns true in case of success.
when defined(windows):
Expand Down Expand Up @@ -2144,7 +2141,7 @@ iterator walkPattern*(pattern: string): string {.tags: [ReadDirEffect], noWeirdT
## * `walkDirRec iterator <#walkDirRec.i,string>`_
runnableExamples:
import std/sequtils
let paths = toSeq(walkPattern("lib/pure/*")) # works on windows too
let paths = toSeq(walkPattern("lib/pure/*")) # works on Windows too
assert "lib/pure/concurrency".unixToNativePath in paths
assert "lib/pure/os.nim".unixToNativePath in paths
walkCommon(pattern, defaultWalkFilter)
Expand All @@ -2163,7 +2160,7 @@ iterator walkFiles*(pattern: string): string {.tags: [ReadDirEffect], noWeirdTar
## * `walkDirRec iterator <#walkDirRec.i,string>`_
runnableExamples:
import std/sequtils
assert "lib/pure/os.nim".unixToNativePath in toSeq(walkFiles("lib/pure/*.nim")) # works on windows too
assert "lib/pure/os.nim".unixToNativePath in toSeq(walkFiles("lib/pure/*.nim")) # works on Windows too
walkCommon(pattern, isFile)

iterator walkDirs*(pattern: string): string {.tags: [ReadDirEffect], noWeirdTarget.} =
Expand All @@ -2180,7 +2177,7 @@ iterator walkDirs*(pattern: string): string {.tags: [ReadDirEffect], noWeirdTarg
## * `walkDirRec iterator <#walkDirRec.i,string>`_
runnableExamples:
import std/sequtils
let paths = toSeq(walkDirs("lib/pure/*")) # works on windows too
let paths = toSeq(walkDirs("lib/pure/*")) # works on Windows too
assert "lib/pure/concurrency".unixToNativePath in paths
walkCommon(pattern, isDir)

Expand Down Expand Up @@ -2278,7 +2275,9 @@ iterator walkDir*(dir: string; relative = false, checkDir = false):
## If `checkDir` is true, `OSError` is raised when `dir`
## doesn't exist.
##
## Example: This directory structure::
## **Example:**
##
## This directory structure::
## dirA / dirB / fileB1.txt
## / dirC
## / fileA1.txt
Expand All @@ -2291,7 +2290,6 @@ iterator walkDir*(dir: string; relative = false, checkDir = false):
# this also works at compile time
assert collect(for k in walkDir("dirA"): k.path).join(" ") ==
"dirA/dirB dirA/dirC dirA/fileA2.txt dirA/fileA1.txt"
##
## See also:
## * `walkPattern iterator <#walkPattern.i,string>`_
## * `walkFiles iterator <#walkFiles.i,string>`_
Expand Down Expand Up @@ -3271,7 +3269,7 @@ template rawToFormalFileInfo(rawInfo, path, formalInfo): untyped =
formalInfo.lastAccessTime = fromWinTime(rdFileTime(rawInfo.ftLastAccessTime))
formalInfo.lastWriteTime = fromWinTime(rdFileTime(rawInfo.ftLastWriteTime))
formalInfo.creationTime = fromWinTime(rdFileTime(rawInfo.ftCreationTime))
formalInfo.blockSize = 8192 # xxx use windows API instead of hardcoding
formalInfo.blockSize = 8192 # xxx use Windows API instead of hardcoding

# Retrieve basic permissions
if (rawInfo.dwFileAttributes and FILE_ATTRIBUTE_READONLY) != 0'i32:
Expand Down Expand Up @@ -3494,27 +3492,30 @@ proc setLastModificationTime*(file: string, t: times.Time) {.noWeirdTarget.} =
discard h.closeHandle
if res == 0'i32: raiseOSError(osLastError(), file)

func isValidFilename*(filename: string, maxLen = 259.Positive): bool {.since: (1, 1).} =
func isValidFilename*(filename: string, maxLen = 259.Positive): bool {.since: (1, 1), deprecated: "Deprecated since v1.5.1".} =
## Returns true if ``filename`` is valid for crossplatform use.
##
## This is useful if you want to copy or save files across Windows, Linux, Mac, etc.
## You can pass full paths as argument too, but func only checks filenames.
## It uses ``invalidFilenameChars``, ``invalidFilenames`` and ``maxLen`` to verify the specified ``filename``.
##
## .. code-block:: nim
## assert not isValidFilename(" foo") ## Leading white space
## assert not isValidFilename("foo ") ## Trailing white space
## assert not isValidFilename("foo.") ## Ends with Dot
## assert not isValidFilename("con.txt") ## "CON" is invalid (Windows)
## assert not isValidFilename("OwO:UwU") ## ":" is invalid (Mac)
## assert not isValidFilename("aux.bat") ## "AUX" is invalid (Windows)
## It uses `invalidFilenameChars`, `invalidFilenames` and `maxLen` to verify the specified `filename`.
##
runnableExamples:
assert not isValidFilename(" foo") # Leading white space
assert not isValidFilename("foo ") # Trailing white space
assert not isValidFilename("foo.") # Ends with dot
assert not isValidFilename("con.txt") # "CON" is invalid (Windows)
assert not isValidFilename("OwO:UwU") # ":" is invalid (Mac)
assert not isValidFilename("aux.bat") # "AUX" is invalid (Windows)
assert not isValidFilename("") # Empty string
assert not isValidFilename("foo/") # Filename is empty

# https://docs.microsoft.com/en-us/dotnet/api/system.io.pathtoolongexception
# https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
# https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx
result = true
let f = filename.splitFile()
if unlikely(f.name.len + f.ext.len > maxLen or
if unlikely(f.name.len + f.ext.len > maxLen or f.name.len == 0 or
f.name[0] == ' ' or f.name[^1] == ' ' or f.name[^1] == '.' or
find(f.name, invalidFilenameChars) != -1): return false
for invalid in invalidFilenames:
Expand Down

0 comments on commit 62701cd

Please sign in to comment.