Skip to content
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

Added a missing case for __std_is_file_not_found(). #5381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tylerbrawl
Copy link
Contributor

I noticed some rather peculiar behavior regarding the Win32 API GetFileAttributesExW(). Specifically, I found that upon calling this function with an invalid path (in my case, a remote URI) in two separate applications, both would fail as expected. What is interesting is that - for some inexplicable reason - the error code returned by GetLastError() differed between these two applications. Specifically, it was always either one of the following:

  • ERROR_INVALID_NAME: The file name, directory name, or volume label syntax is incorrect.

  • ERROR_BAD_PATHNAME: The specified path is invalid.

When this occurred, I was making use of the throwing std::filesystem::exists() overload. So, you could imagine my surprise when this function threw an exception on one of the applications and returned false as expected on the other. I have not spent any real time investigating the cause for this difference in behavior myself. In any case, though, it is clear that the __std_is_file_not_found() function should have returned true when given an __std_win_error value corresponding to either of the two error codes.

This commit thus modifies the __std_is_file_not_found() function to take into consideration an additional Win32 error code: ERROR_BAD_PATHNAME. Prior to this commit, this value did not have a unique value assigned to it in the __std_win_error enum class. Thus, a new value by the name of __std_win_error::_Bad_pathname was also added in this commit.

I noticed some rather peculiar behavior regarding the Win32 API
`GetFileAttributesExW()`. Specifically, I found that upon calling
this function with an invalid path (in my case, a remote URI) in
two separate applications, both would fail as expected. What is
interesting is that - for some inexplicable reason - the error code
returned by `GetLastError()` differed between these two
applications. Specifically, it was always either one of the
following:

  * `ERROR_INVALID_NAME`: The file name, directory name, or volume
    label syntax is incorrect.

  * `ERROR_BAD_PATHNAME`: The specified path is invalid.

When this occurred, I was making use of the throwing
`std::filesystem::exists()` overload. So, you could imagine my
surprise when this function threw an exception on one of the
applications and returned `false` as expected on the other. I have
not spent any real time investigating the cause for this difference
in behavior myself. In any case, though, it is clear that the
`__std_is_file_not_found()` function should have returned `true`
when given an `__std_win_error` value corresponding to either of
the two error codes.

This commit thus modifies the `__std_is_file_not_found()` function
to take into consideration an additional Win32 error code:
`ERROR_BAD_PATHNAME`. Prior to this commit, this value did not have
a unique value assigned to it in the `__std_win_error`
`enum class`. Thus, a new value by the name of
`__std_win_error::_Bad_pathname` was also added in this commit.
@tylerbrawl tylerbrawl requested a review from a team as a code owner March 31, 2025 14:41
tylerbrawl added a commit to tylerbrawl/rocky that referenced this pull request Mar 31, 2025
…::URI::read()`.

There appears to be a bug in the MSVC STL implementation of
`std::filesystem::exists()` in which the mechanism used to
determine whether or not a given file is found (that is, the
`__std_is_file_not_found()` function, although that is not a part
of the C++ Standard) does not consider every possible relevant
Win32 error code which could be returned by
`GetFileAttributesExW()`. (This issue has been documented in a pull
request at microsoft/STL#5381.)

This commit implements a work around for that issue by modifying
the `rocky::URI::read()` function to use the non-throwing variant
of `std::filesystem::exists()` in order to determine whether the
URI returned from `rocky::URI::full()` represents an existing file
in the local filesystem. This variant of the function will return
`false` if the specified `std::filesystem::path` does not represent
an existing object in the filesystem, just like the throwing
variant. In addition, it will also return `false` if an error
prevented the function from determining whether or not the file
exists; this effectively handles the fact that the MSVC STL does
not currently respect the `ERROR_BAD_PATHNAME` Win32 error code.
@StephanTLavavej StephanTLavavej added bug Something isn't working filesystem C++17 filesystem labels Mar 31, 2025
@StephanTLavavej
Copy link
Member

Sounds a lot like #4844 and #5077.

I have not spent any real time investigating the cause for this difference in behavior myself.

We probably don't need to understand the root cause in order to merge this change, but are there any interesting differences between the systems? For example, are they running different OSes, like the Win11 24H2 changes above?

@StephanTLavavej StephanTLavavej self-assigned this Mar 31, 2025
@tylerbrawl
Copy link
Contributor Author

Both applications were running on the same system, which is running Windows 11 24H2, as you suspect. I don't really know what could cause the difference in behavior, honestly. I am a bit busy at work right now, so I wouldn't be able to investigate it until a little later.

@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej
Copy link
Member

Aha, I believe you've fixed DevCom-10825862 VSO-2345522 "std::filesystem::status throws on bad paths when the current directory is a network path" which matches your description, but also describes the necessary environment.

With VS 2022 17.14 Preview 2, I repro:

C:\Temp>type meow.cpp
#include <filesystem>
#include <print>
using namespace std;

int main() {
    try {
        println("_MSVC_STL_UPDATE: {}", _MSVC_STL_UPDATE);
        const bool b = filesystem::exists(R"(https://example.com/bogus/path)");
        println("{}", b);
    } catch (const filesystem::filesystem_error& e) {
        println("{}", e.what());
    }
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp && meow
meow.cpp
_MSVC_STL_UPDATE: 202501
false

C:\Temp>pushd \\localhost\C$\Temp

Y:\Temp>meow
_MSVC_STL_UPDATE: 202501
exists: The specified path is invalid.: "https://example.com/bogus/path"

I observe that your fix is effective:

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp && meow
meow.cpp
_MSVC_STL_UPDATE: 202503
false

C:\Temp>pushd \\localhost\C$\Temp

X:\Temp>meow
_MSVC_STL_UPDATE: 202503
false

Awesome! 😻

@StephanTLavavej StephanTLavavej removed their assignment Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working filesystem C++17 filesystem
Projects
Status: Ready To Merge
Development

Successfully merging this pull request may close these issues.

2 participants