danreeves / path-clean

A Rust implementation of `cleanname` or `path.Clean`

path-clean's Issues

Is this safe?

Hello,

There is ongoing discussion for sanitizing paths in typst, I was wondering if you consider this implementation to be acceptable as a safety feature? For example to check whether a script is trying to access a file outside of the predetermined root directory?

Thanks in advance,

Dherse

Consider adding #![forbid(unsafe_code)]

cargo-geiger marks crates in one of three ways:

• Red for crates using unsafe and, thus, in need of more careful and skilled auditing.
• Uncoloured for crates wth no unsafe
• Green for crates which use #![forbid(unsafe_code)]

Given that the point of path-clean is to "[perform] this transform lexically, without touching the filesystem" (unlike normpath which depends on GetFullPathNameW on Windows), adding #![forbid(unsafe_code)] as an additional indicator of the intent to do it without FFI calls seems like a good idea.

Path containing back slashes and forward slashes is not cleaned

This pathbuf is not cleaned on windows:

PathBuf::from("/dir\\../otherDir/test.json").clean()

Remains as:

/dir\../otherDir/test.json

Workaround:

PathBuf::from(file_path.to_string_lossy().replace("\\", "/")).clean()

Don't assume valid UTF-8

Currently, your clean function requires &str and your trait just throws out any paths containing invalid UTF-8.

Given how easily one can have invalid UTF-8 on POSIX platforms from mojibake in filenames (eg. Load data onto a mobile device that uses latin1 on-disk using its USB Mass Storage Device mode, then pull the SD card out and plug it into a UTF-8 Linux device directly), this makes it of very limited use. (And Windows also allows unpaired UTF-16 surrogates in filenames for historical reasons.)

The proper solution would be to use the Path::components iterator so you don't need to convert to &str to match on component types or, for that matter, even think about what path separator the platform is using.

You'd just get a sequence of Prefix(PrefixComponent) (Stuff like C: or \\server\share), RootDir, CurDir, ParentDir, and Normal(&OsStr) values, and it'd collapse away //, /./, and trailing / and /. for you.

(That'd also allow you to eliminate an unnecessary intermediate copy in going from PathBuf to &str, since you can just impl your trait on Path.)

clean("c:/temp/..") results in "c:", not "c:/"

You would expect "c:/temp/.." to be normalized to "c:/", but it results in "c:". "c:" does not mean the root of windows file system, but rather current directory in drive c:.

    let buf = clean("c:/temp/..");
    assert_eq!(buf, "c:");