Skip to content

Unreachable code reached when using UNC paths on Windows #8205

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

Closed
JerwuQu opened this issue Mar 11, 2021 · 4 comments · Fixed by #15768
Closed

Unreachable code reached when using UNC paths on Windows #8205

JerwuQu opened this issue Mar 11, 2021 · 4 comments · Fixed by #15768
Labels
bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@JerwuQu
Copy link
Contributor

JerwuQu commented Mar 11, 2021

Using absolute paths related to network shares (starting with \\...) will result in stdlib reaching unreachable, meaning it's impossible to recover from. Preferred would be if it spat an error such as error.UnsupportedDrive so it can be handled gracefully, or if support for them was added.

Note that getting the working dir handle from std.fs.cwd() whilst on a network share works, and you can use openDir, openFile, and all other relative methods from there. Meaning if you've somehow gotten a handle, you can access and read/write files. Afaik it only breaks when you try to use absolute paths.

Getting working dir from std.fs.cwd() whilst on a network share and then running realpath on the handle also breaks with unreachable.

I've only experienced this because I've done development in WSL2, which Windows has mounted as a network share.

Example:

const std = @import("std");

pub fn main() !void {
    const path = "\\\\wsl$\\Arch\\home\\jerwuqu";
    std.debug.print("Is absolute: {}\n", .{std.fs.path.isAbsolute(path)});

    _ = try std.fs.openDirAbsolute(path, .{});
    std.debug.print("OK!\n", .{});
}

Output:

Is absolute: true
thread 19092 panic: reached unreachable code
Panicked during a panic. Aborting.
@SpexGuy SpexGuy added bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library. labels Mar 16, 2021
@Vexu Vexu added this to the 0.8.0 milestone Mar 19, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 0.8.1 Jun 4, 2021
@andrewrk andrewrk modified the milestones: 0.8.1, 0.9.1 Aug 31, 2021
@andrewrk andrewrk modified the milestones: 0.9.1, 0.9.0, 0.10.0 Nov 20, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@star-tek-mb
Copy link
Contributor

std.c.fopen can open files on WSL, but std.fs.openFileAbsolute can't with \\wsl.localhost\Debian like path

@star-tek-mb
Copy link
Contributor

star-tek-mb commented Nov 14, 2022

For files on network share we must use UNC:

var content = try std.fs.cwd().readFileAlloc(allocator, "\\??\\UNC\\wsl.localhost\\Debian\\home\\noobtek\\main.c", std.math.maxInt(usize));

For local files UNC also can be used:

var content = try std.fs.cwd().readFileAlloc(allocator, "\\??\\UNC\\localhost\\D$\\main.c", std.math.maxInt(usize));

\\??\\UNC\\localhost\\D:\\main.c - doesn't work.

So, I think we need to handle these edge cases, at least on std.debug on stack traces.

nolanderc added a commit to nolanderc/zig that referenced this issue Apr 14, 2023
Fixes ziglang#8205 and ziglang#12729.

The old method of prepending `\??\` at the start of the DOS path
produced invalid strings for UNC paths (such as those found in WSL).
Instead, we rely on the conversion routine in ntdll, which should handle
all current and future edge cases properly.
Vexu pushed a commit to nolanderc/zig that referenced this issue Apr 23, 2023
Fixes ziglang#8205 and ziglang#12729.

The old method of prepending `\??\` at the start of the DOS path
produced invalid strings for UNC paths (such as those found in WSL).
Instead, we rely on the conversion routine in ntdll, which should handle
all current and future edge cases properly.
squeek502 added a commit to squeek502/zig that referenced this issue May 19, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as rooted and resolved against the current drive (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes ziglang#8205
Might close (untested) ziglang#12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to ziglang#15607
squeek502 added a commit to squeek502/zig that referenced this issue May 19, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Drive-relative paths (`C:foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as rooted and resolved against the current drive (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes ziglang#8205
Might close (untested) ziglang#12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to ziglang#15607
squeek502 added a commit to squeek502/zig that referenced this issue May 19, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Drive-relative paths (`C:foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as 'drive relative' and resolved against the CWD (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes ziglang#8205
Might close (untested) ziglang#12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to ziglang#15607
Vexu pushed a commit that referenced this issue May 29, 2023
…aths

There are many different types of Windows paths, and there are a few different possible namespaces on top of that. Before this commit, NT namespaced paths were somewhat supported, and for Win32 paths (those without a namespace prefix), only relative and drive absolute paths were supported. After this commit, all of the following are supported:

- Device namespaced paths (`\\.\`)
- Verbatim paths (`\\?\`)
- NT-namespaced paths (`\??\`)
- Relative paths (`foo`)
- Drive-absolute paths (`C:\foo`)
- Drive-relative paths (`C:foo`)
- Rooted paths (`\foo`)
- UNC absolute paths (`\\server\share\foo`)
- Root local device paths (`\\.` or `\\?` exactly)

Plus:

- Any of the path types and namespace types can be mixed and matched together as appropriate.
- All of the `std.os.windows.*ToPrefixedFileW` functions will accept any path type, prefixed or not, and do the appropriate thing to convert them to an NT-prefixed path if necessary.

This is achieved by making the `std.os.windows.*ToPrefixedFileW` functions behave like `ntdll.RtlDosPathNameToNtPathName_U`, but with a few differences:

- Does not allocate on the heap (this is why we can't use `ntdll.RtlDosPathNameToNtPathName_U` directly, it does internal heap allocation).
- Relative paths are kept as relative unless they contain too many .. components, in which case they are treated as 'drive relative' and resolved against the CWD (this is how it behaved before this commit as well).
- Special case device names like COM1, NUL, etc are not handled specially (TODO)
- `.` and space are not stripped from the end of relative paths (potential TODO)

Most of the non-trivial conversion of non-relative paths is done via `ntdll.RtlGetFullPathName_U`, which AFAIK is used internally by `ntdll.RtlDosPathNameToNtPathName_U`.

Some relevant reading on Windows paths:

- https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html
- https://chrisdenton.github.io/omnipath/Overview.html

Closes #8205
Might close (untested) #12729

Note:
- This removes checking for illegal characters in `std.os.windows.sliceToPrefixedFileW`, since the previous solution (iterate the whole string and error if any illegal characters were found) was naive and won't work for all path types. This is further complicated by things like file streams (where `:` is used as a delimiter, e.g. `file.ext:stream_name:$DATA`) and things in the device namespace (where a path like `\\.\GLOBALROOT\??\UNC\localhost\C$\foo` is valid despite the `?`s in the path and is effectively equivalent to `C:\foo`). Truly validating paths is complicated and would need to be tailored to each path type. The illegal character checking being removed may open up users to more instances of hitting `OBJECT_NAME_INVALID => unreachable` when using `fs` APIs.
  + This is related to #15607
@andrewrk
Copy link
Member

Fix reverted in b5fad3a due to master branch CI failures

@andrewrk andrewrk reopened this May 29, 2023
@squeek502
Copy link
Collaborator

squeek502 commented Jun 4, 2023

This can be re-closed since the fix was reinstated in #15905

@andrewrk andrewrk closed this as completed Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior os-windows standard library This issue involves writing Zig code for the standard library.
Projects
None yet
6 participants