<filesystem>::space fails with no read access to root (#552)

* <filesystem>::space fails with no read access to root

Avoid the unnecessary call to GetVolumePathNameW because read access for this call is required on the root of the mounted volume. This call used to be required for GetDiskFreeSpace, but should not be used for GetDiskFreeSpaceExW, where the full path should be used. The appropriate read permissions on the given path will be taken into consideration.

Co-authored-by: Billy O'Neal <billy.oneal@gmail.com>

stl/CMakeLists.txt

@@ -238,7 +238,6 @@ endforeach()
 # Objs that exist in both libcpmt[d][01].lib and msvcprt[d].lib.
 set(IMPLIB_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/filesystem.cpp
-    ${CMAKE_CURRENT_LIST_DIR}/src/filesystem_space.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/locale0_implib.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/nothrow.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/parallel_algorithms.cpp

stl/msbuild/stl_base/stl.files.settings.targets

@@ -168,7 +168,6 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
              (controlled by IncludeInLink and IncludeInImportLib). -->
         <BuildFiles Include="
             $(CrtRoot)\github\stl\src\filesystem.cpp;
-            $(CrtRoot)\github\stl\src\filesystem_space.cpp;
             $(CrtRoot)\github\stl\src\locale0_implib.cpp;
             $(CrtRoot)\github\stl\src\nothrow.cpp;
             $(CrtRoot)\github\stl\src\parallel_algorithms.cpp;

stl/src/filesystem.cpp

@@ -748,6 +748,83 @@ __std_win_error __stdcall __std_fs_get_file_id(__std_fs_file_id* const _Id, cons
     return __std_win_error::_Success;
 }
 
+[[nodiscard]] __std_win_error __stdcall __std_fs_space(const wchar_t* const _Target, uintmax_t* const _Available,
+    uintmax_t* const _Total_bytes, uintmax_t* const _Free_bytes) noexcept {
+    // get capacity information for the volume on which the file _Target resides
+    static_assert(sizeof(uintmax_t) == sizeof(ULARGE_INTEGER) && alignof(uintmax_t) == alignof(ULARGE_INTEGER),
+        "Size and alignment must match for reinterpret_cast<PULARGE_INTEGER>");
+    const auto _Available_c   = reinterpret_cast<PULARGE_INTEGER>(_Available);
+    const auto _Total_bytes_c = reinterpret_cast<PULARGE_INTEGER>(_Total_bytes);
+    const auto _Free_bytes_c  = reinterpret_cast<PULARGE_INTEGER>(_Free_bytes);
+    if (GetDiskFreeSpaceExW(_Target, _Available_c, _Total_bytes_c, _Free_bytes_c)) {
+        return __std_win_error::_Success;
+    }
+
+    __std_win_error _Last_error{GetLastError()};
+    _Available_c->QuadPart   = ~0ull;
+    _Total_bytes_c->QuadPart = ~0ull;
+    _Free_bytes_c->QuadPart  = ~0ull;
+    if (_Last_error != __std_win_error::_Directory_name_is_invalid) {
+        return _Last_error;
+    }
+
+    // Input could have been a file; canonicalize and remove the last component.
+    // We use VOLUME_NAME_NT because it always has a mapping available and we don't care about the canonical path
+    // being "ugly" due to needing the _Dos_to_nt_prefix.
+    static constexpr wchar_t _Dos_to_nt_prefix[] = LR"(\\?\GLOBALROOT)";
+    constexpr size_t _Dos_to_nt_prefix_count     = sizeof(_Dos_to_nt_prefix) / sizeof(wchar_t) - 1;
+    __crt_unique_heap_ptr<wchar_t> _Buf;
+    DWORD _Actual_length;
+    {
+        const _STD _Fs_file _Handle(
+            _Target, __std_access_rights::_File_read_attributes, __std_fs_file_flags::_Backup_semantics, &_Last_error);
+        if (_Last_error != __std_win_error::_Success) {
+            return _Last_error;
+        }
+
+        DWORD _Buf_count = MAX_PATH;
+        for (;;) {
+            _Buf = _malloc_crt_t(wchar_t, _Buf_count);
+            if (!_Buf) {
+                return __std_win_error::_Not_enough_memory;
+            }
+
+            _Actual_length = __vcrt_GetFinalPathNameByHandleW(_Handle._Get(), _Buf.get() + _Dos_to_nt_prefix_count,
+                _Buf_count - _Dos_to_nt_prefix_count, FILE_NAME_NORMALIZED | VOLUME_NAME_NT);
+            if (_Actual_length == 0) {
+                return __std_win_error{GetLastError()};
+            }
+
+            _Actual_length += _Dos_to_nt_prefix_count;
+            if (_Actual_length <= _Buf_count) {
+                break;
+            }
+
+            _Buf_count = _Actual_length;
+        }
+    } // close _Handle
+
+    const auto _Ptr = _Buf.get();
+
+    memcpy(_Ptr, _Dos_to_nt_prefix, _Dos_to_nt_prefix_count * sizeof(wchar_t));
+
+    // insert null terminator at the last slash
+    auto _Cursor = _Ptr + _Actual_length;
+    do {
+        --_Cursor; // cannot run off start because _Dos_to_nt_prefix contains a backslash
+    } while (*_Cursor != L'\\');
+
+    *_Cursor = L'\0';
+    if (GetDiskFreeSpaceExW(_Ptr, _Available_c, _Total_bytes_c, _Free_bytes_c)) {
+        return __std_win_error::_Success;
+    }
+
+    _Available_c->QuadPart   = ~0ull;
+    _Total_bytes_c->QuadPart = ~0ull;
+    _Free_bytes_c->QuadPart  = ~0ull;
+    return __std_win_error{GetLastError()};
+}
+
 [[nodiscard]] __std_ulong_and_error __stdcall __std_fs_get_temp_path(wchar_t* const _Target) noexcept {
     // calls GetTempPathW
     // If getting the path failed, returns 0 size; otherwise, returns the size of the

stl/src/filesystem_space.cpp

@@ -1,73 +0,0 @@
-// Copyright (c) Microsoft Corporation.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-// This must be as small as possible, because its contents are
-// injected into the msvcprt.lib and msvcprtd.lib import libraries.
-// Do not include or define anything else here.
-// In particular, basic_string must not be included here.
-
-// TRANSITION, the code in this file should be moved back to filesystem.cpp
-// when a Windows 10 SDK beyond version 1903 is available (see GH-322).
-
-#include <internal_shared.h>
-#include <limits.h>
-#include <xfilesystem_abi.h>
-
-#include <Windows.h>
-
-namespace {
-    static_assert(sizeof(uintmax_t) == sizeof(ULARGE_INTEGER) && alignof(uintmax_t) == alignof(ULARGE_INTEGER),
-        "Size and alignment must match for reinterpret_cast<PULARGE_INTEGER>");
-
-    [[nodiscard]] __std_win_error _Fs_space_attempt(wchar_t* const _Temp_buffer, const DWORD _Temp_buffer_characters,
-        const wchar_t* const _Target, uintmax_t* const _Available, uintmax_t* const _Total_bytes,
-        uintmax_t* const _Free_bytes) noexcept {
-        if (GetVolumePathNameW(_Target, _Temp_buffer, _Temp_buffer_characters)) {
-            if (GetDiskFreeSpaceExW(_Temp_buffer, reinterpret_cast<PULARGE_INTEGER>(_Available),
-                    reinterpret_cast<PULARGE_INTEGER>(_Total_bytes), reinterpret_cast<PULARGE_INTEGER>(_Free_bytes))) {
-                return __std_win_error::_Success;
-            }
-        }
-
-        return __std_win_error{GetLastError()};
-    }
-} // unnamed namespace
-
-_EXTERN_C
-[[nodiscard]] __std_win_error __stdcall __std_fs_space(const wchar_t* const _Target, uintmax_t* const _Available,
-    uintmax_t* const _Total_bytes, uintmax_t* const _Free_bytes) noexcept {
-    // get capacity information for the volume on which the file _Target resides
-    __std_win_error _Last_error;
-    if (GetFileAttributesW(_Target) == INVALID_FILE_ATTRIBUTES) {
-        _Last_error = __std_win_error{GetLastError()};
-    } else {
-        {
-            constexpr DWORD _Static_size = MAX_PATH;
-            wchar_t _Temp_buf[_Static_size];
-            _Last_error = _Fs_space_attempt(_Temp_buf, _Static_size, _Target, _Available, _Total_bytes, _Free_bytes);
-            if (_Last_error == __std_win_error::_Success) {
-                return __std_win_error::_Success;
-            }
-        }
-
-        if (_Last_error == __std_win_error::_Filename_exceeds_range) {
-            constexpr DWORD _Dynamic_size = USHRT_MAX + 1; // assuming maximum NT path fits in a UNICODE_STRING
-            const auto _Temp_buf          = _malloc_crt_t(wchar_t, _Dynamic_size);
-            if (_Temp_buf) {
-                _Last_error =
-                    _Fs_space_attempt(_Temp_buf.get(), _Dynamic_size, _Target, _Available, _Total_bytes, _Free_bytes);
-                if (_Last_error == __std_win_error::_Success) {
-                    return __std_win_error::_Success;
-                }
-            } else {
-                _Last_error = __std_win_error::_Not_enough_memory;
-            }
-        }
-    }
-
-    *_Available   = ~0ull;
-    *_Total_bytes = ~0ull;
-    *_Free_bytes  = ~0ull;
-    return _Last_error;
-}
-_END_EXTERN_C

tests/std/tests/P0218R1_filesystem/test.cpp

@@ -3354,12 +3354,25 @@ void test_space() {
     EXPECT(info.free == maxValue);
     EXPECT(info.capacity == maxValue);
 
+    const path nonexistent(dir / L"nonexistent"sv);
+    info = space(nonexistent, ec);
+    EXPECT(bad(ec));
+    EXPECT(info.available == maxValue);
+    EXPECT(info.free == maxValue);
+    EXPECT(info.capacity == maxValue);
+
     info = space(LR"(C:\Some\Path\That\Does\Not\Exist)", ec);
     EXPECT(bad(ec));
     EXPECT(info.available == maxValue);
     EXPECT(info.free == maxValue);
     EXPECT(info.capacity == maxValue);
 
+    info = space(LR"(??malformed??)", ec);
+    EXPECT(bad(ec));
+    EXPECT(info.available == maxValue);
+    EXPECT(info.free == maxValue);
+    EXPECT(info.capacity == maxValue);
+
     // Also test VSO-732321 [Feedback] Filesystem::space() doesn't work if the parameter is a relative path
     info = space(L"."sv, ec);
     EXPECT(good(ec));
@@ -3367,9 +3380,43 @@ void test_space() {
     EXPECT(info.free != maxValue);
     EXPECT(info.capacity != maxValue);
 
-    remove(file, ec);
+    const path symDirPresent(dir / L"directory_symlink"sv);
+    const path symDirAbsent(dir / L"broken_directory_symlink"sv);
+    const path symFilePresent(dir / L"file_symlink"sv);
+    const path symFileAbsent(dir / L"broken_file_symlink"sv);
+
+    if ((create_directory_symlink(current_path() / dir, symDirPresent, ec), ec)
+        || (create_directory_symlink(nonexistent, symDirAbsent, ec), ec)
+        || (create_symlink(current_path() / file, symFilePresent, ec), ec)
+        || (create_symlink(nonexistent, symFileAbsent, ec), ec)) {
+        check_symlink_permissions(ec, L"space symlink edge cases");
+    } else {
+        info = space(symDirPresent, ec);
         EXPECT(good(ec));
-    remove(dir, ec);
+        EXPECT(info.available != maxValue);
+        EXPECT(info.free != maxValue);
+        EXPECT(info.capacity != maxValue);
+
+        info = space(symFilePresent, ec);
+        EXPECT(good(ec));
+        EXPECT(info.available != maxValue);
+        EXPECT(info.free != maxValue);
+        EXPECT(info.capacity != maxValue);
+
+        info = space(symDirAbsent, ec);
+        EXPECT(bad(ec));
+        EXPECT(info.available == maxValue);
+        EXPECT(info.free == maxValue);
+        EXPECT(info.capacity == maxValue);
+
+        info = space(symFileAbsent, ec);
+        EXPECT(bad(ec));
+        EXPECT(info.available == maxValue);
+        EXPECT(info.free == maxValue);
+        EXPECT(info.capacity == maxValue);
+    }
+
+    remove_all(dir, ec);
     EXPECT(good(ec));
 }