. unhelpfully normalized to "C:" when current directory is "C:\"

[gbarta]

I am not a rust dev, just a user of the fd utility and this relates to a bug I found sharkdp/fd#931

It seems that when the current directory is "C:\" the normalize function of this library will transform "." into "C:".

With the following test app:

use normpath::PathExt;
use std::env;
use std::path::Path;
fn main() {
    let dot = Path::new(".");

    let root = Path::new("c:\\");
    assert!(env::set_current_dir(&root).is_ok());
    let norm_dot = dot.normalize().unwrap();
    let norm_dot_path = norm_dot.as_path();
    println!("With current dir {} {} normalized to {}", root.display(), dot.display(), norm_dot_path.display());

    let users = Path::new("C:\\Users");
    assert!(env::set_current_dir(&users).is_ok());
    let norm_dot = dot.normalize().unwrap();
    let norm_dot_path = norm_dot.as_path();
    println!("With current dir {} {} normalized to {}", users.display(), dot.display(), norm_dot_path.display());
}

we get:

C:\TEMP\fd\normpath_issue>cargo build && target\debug\normpath_issue.exe
   Compiling normpath_issue v0.1.0 (C:\TEMP\fd\normpath_issue)
    Finished dev [unoptimized + debuginfo] target(s) in 0.74s
With current dir c:\ . normalized to c:
With current dir C:\Users . normalized to C:\Users

I think it would be much better if it returned C:\ instead, with the trailing separator. On windows, C: does not represent any specific directory, rather it represents the current directory associated with the C drive. It is a common misconception that C: is the same as C:\ but this is not the case.

[gbarta]

For what it is worth, the equivalent in python and node do not attempt to transform . at all:

C:\Windows>cd \

C:\>python
Python 3.10.1 (tags/v3.10.1:2cd268a, Dec  6 2021, 19:10:37) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os.path
>>> os.path.normpath(".")
'.'
>>> exit()

C:\>node
Welcome to Node.js v17.3.0.
Type ".help" for more information.
> path.normalize(".")
'.'

[dylni]

Thank you for reporting this! Strangely, WinAPI returns c: for this example instead of resolving the path, which is why this crate returns the same value. I agree that c:\ should be returned instead and will add a workaround to resolve this issue.

For what it is worth, the equivalent in python and node do not attempt to transform . at all:

Unfortunately, those functions are deceptively named. They are meant to simplify paths, rather than normalizing them, and can return incorrect results.

For example, on Unix, they transform foo/../zoo into zoo, which is not always correct. If foo is a symlink to bar/baz, the correct normalized path is bar/zoo. zoo would be a different path.

This crate asks the operating system for the normalized path, so it should be able to handle strange paths correctly.

[gbarta]

Thanks! Browsing the source, it looked to me like the heavy lifting was done by GetFullPathNameW, although that appeared to be doing the expected thing, so I wasn't really sure what was going on:

C:\Windows>python
Python 3.10.1 (tags/v3.10.1:2cd268a, Dec  6 2021, 19:10:37) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os, ctypes
>>> os.chdir("C:\\")
>>> buf=ctypes.create_unicode_buffer(260)
>>> rv=ctypes.windll.kernel32.GetFullPathNameW(".",260,buf,None)
>>> print(buf.value)
C:\

[dylni]

That's the right function, and thank you for the example. It clearly demonstrates that this is a bug in GetFullPathNameW. If you use the size of the buffer it suggests to allocate, the incorrect result is returned:

>>> rv=ctypes.windll.kernel32.GetFullPathNameW(".",3,buf,None)
>>> print(buf.value)
C:

Thus, allocating a larger buffer should be enough to easily resolve this issue, but I'll check if that causes any other issues.

[dylni]

Version 0.3.2 should fix this issue. Thank you for the detailed bug report and comments!