classification
Title: [Windows] Inconsistent os.stat behavior for directory with Access Denied
Type: behavior Stage: resolved
Components: Extension Modules, Windows Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: CrouZ, eryksun, miss-islington, nw0, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2019-11-16 18:14 by CrouZ, last changed 2021-04-22 23:56 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25527 merged steve.dower, 2021-04-22 19:02
PR 25530 merged miss-islington, 2021-04-22 19:45
PR 25531 merged miss-islington, 2021-04-22 19:45
PR 25540 merged steve.dower, 2021-04-22 22:39
PR 25543 merged miss-islington, 2021-04-22 23:34
PR 25542 merged miss-islington, 2021-04-22 23:34
Messages (12)
msg356763 - (view) Author: (CrouZ) Date: 2019-11-16 18:14
After upgrading some scripts from Python 2.7 to 3.7 in Windows 10, I got different behavior that seems to be caused by inconsistent behavior for os.stat in Python 3.7.

Python 2.7:
>>> os.stat("D:\\System Volume Information")
nt.stat_result ...
>>> os.stat("D:\\System Volume Information\\")
nt.stat_result ... (same as previous call)

Python 3.7:
>>> os.stat("D:\\System Volume Information")
os.stat_result ...
>>> os.stat("D:\\System Volume Information\\")
Traceback ...
PermissionError: [WinError 5] Access is denied: 'D:\\System Volume Information\\'



What I really do is calling:
>>> os.path.exists("D:\\System Volume Information\\")
False (Unexpected and inconsistent. I expect the return value to be True.)

Behavior for other calls:
>>> os.path.exists("D:\\System Volume Information")
True (OK)
>>> os.path.isdir("D:\\System Volume Information\\")
True (OK, but according to the documentation "Return True if path is an existing directory." where 'existing' links to os.path.exists, which returns False)

The closest issue I could find was Issue28075 which has already been fixed.
msg356885 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-18 17:03
I haven't debugged it, but I'm guessing we're not handling the trailing slash properly when falling back to asking the parent directory for information.

Looking at the actual stat result for "C:\System Volume Information" vs. "C:\Windows", most of the information is missing in the first case, which should mean that we can't access it but we know it's a directory because the entry in C:\ said so.
msg356915 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-11-18 21:53
In attributes_from_dir() in Modules/posixmodule.c, a trailing backslash or slash should be stripped from the lpFileName parameter of FindFirstFileW. Otherwise the call opens the directory via NtOpenFile, instead of opening its parent directory. Even if opening the directory is successful, which we don't expect in this case, FindFirstFileW forcibly fails the call with ERROR_FILE_NOT_FOUND (2) because it expects a filename filter (e.g. "*") for the internal NtQueryDirectoryFile[Ex] system call.

Care needs to be taken to not strip the trailing slash of the root directory of a DOS drive because that creates a drive-relative path (e.g. "C:"). It is expected that FindFirstFileW will fail for the root of a DOS drive or UNC share, since there's no parent directory to open.

----

"System Volume Information" explicitly grants access only to the SYSTEM account. Implicitly we have read-attributes access to this directory because we have read-data (i.e. list-directory) access to the root directory. Great, but even for 0 desired access, CreateFileW requests both read-attributes and synchronize access, even for overlapped I/O (i.e. kernel File objects created by CreateFileW can always be waited on). So even an elevated administrator normally can't open this directory to query information. However, backup and restore privileges are in effect when an open requests backup semantics, which we already do. We could extend os.stat to temporarily enable SeBackupPrivilege if the caller has it. 

It's also possible to open the directory with a native NtOpenFile or NtCreateFile system call, without the FILE_SYNCHRONOUS_IO_NONALERT option and without requesting SYNCHRONIZE access -- i.e. the File object will be asynchronous and not waitable.
msg384729 - (view) Author: (CrouZ) Date: 2021-01-09 14:44
The problem exists in Python 3.8 as well, with the difference that ``os.path.isdir("D:\\System Volume Information\\")`` also returns False.

Tested with Python 3.8.2 and 3.8.7.
msg387445 - (view) Author: (CrouZ) Date: 2021-02-21 09:24
The problem exists in Python 3.9 as well, and it behaves the same as Python 3.8.

Tested with Python 3.9.1.
msg387504 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-02-22 06:49
Here's an implementation of attributes_from_dir() that strips trailing slashes (e.g. "C:/spam///" -> "C:/spam"), which entails copying the const string up to the last character that's not a slash. Rooted paths and drive paths that reference a root directory, such as "/" and "C:////", are special cased to avoid creating an empty path or a drive-relative path (e.g. "C:", which expands to the current working directory on drive C:).

static BOOL
attributes_from_dir(LPCWSTR pszFile, BY_HANDLE_FILE_INFORMATION *info,
                    ULONG *reparse_tag)
{
    BOOL result = TRUE;
    HANDLE hFind;
    WIN32_FIND_DATAW fileData;
    LPWSTR filename = (LPWSTR)pszFile;
    size_t len, n;

    /* Issue 38822: trailing slashes must be removed. */
    /* Get the length without trailing slashes. */
    n = len = wcslen(filename);
    while (n && (filename[n - 1] == L'\\' || filename[n - 1] == L'/')) {
        n--;
    }

    if (n != len) {
        if (n == 0 || (n == 2 && filename[1] == L':')) {
            /* A root directory such as \ or C:\ has no parent to query. */
            result = FALSE;
            goto cleanup;
        }
        /* Copy the string without trailing slashes. */
        filename = malloc((n + 1) * sizeof(WCHAR));
        if (!filename || wcsncpy_s(filename, n + 1, pszFile, n)) {
            result = FALSE;
            goto cleanup;
        }
    }

    hFind = FindFirstFileW(filename, &fileData);
    if (hFind == INVALID_HANDLE_VALUE) {
        result = FALSE;
        goto cleanup;
    }
    FindClose(hFind);
    find_data_to_file_info(&fileData, info, reparse_tag);

cleanup:
    if (filename && filename != pszFile) {
        free(filename);
    }
    return result;
}
msg391625 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-04-22 19:45
New changeset fe63a401a9b3ca1751b81b5d6ddb2beb7f3675c1 by Steve Dower in branch 'master':
bpo-38822: Fixed os.stat failing on inaccessible directories. (GH-25527)
https://github.com/python/cpython/commit/fe63a401a9b3ca1751b81b5d6ddb2beb7f3675c1
msg391628 - (view) Author: miss-islington (miss-islington) Date: 2021-04-22 20:07
New changeset 400bd9a3858ea28ea264682b7f050c69638b1ff1 by Miss Islington (bot) in branch '3.8':
bpo-38822: Fixed os.stat failing on inaccessible directories. (GH-25527)
https://github.com/python/cpython/commit/400bd9a3858ea28ea264682b7f050c69638b1ff1
msg391629 - (view) Author: miss-islington (miss-islington) Date: 2021-04-22 20:10
New changeset 8e7cebb497e6004715a5475f6b53d8ef9d30a9fa by Miss Islington (bot) in branch '3.9':
bpo-38822: Fixed os.stat failing on inaccessible directories. (GH-25527)
https://github.com/python/cpython/commit/8e7cebb497e6004715a5475f6b53d8ef9d30a9fa
msg391632 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-04-22 20:30
Steve, your PR checks `filename[n] == L':'`. I think it should check for a drive name, i.e. `n == 1 && filename[1] == L':'`. For example, the VirtualBox shared-folder filesystem allows creating and accessing filenames that contain and end with colons (e.g. "spam:" or even "./X:"), and such names can be queried with FindFirstFileW(). As long as a filename can be queried, I think it should be supported by attributes_from_dir().
msg391641 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-04-22 23:48
New changeset 9f0b3a9c8eedf694377d8638365fb9f385daa581 by Miss Islington (bot) in branch '3.8':
bpo-38822: Check specifically for a drive, not just a colon (GH-25540)
https://github.com/python/cpython/commit/9f0b3a9c8eedf694377d8638365fb9f385daa581
msg391642 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-04-22 23:56
New changeset 28575923a9ee40928a9d00a7ed997a6f6a09b8d1 by Miss Islington (bot) in branch '3.9':
bpo-38822: Check specifically for a drive, not just a colon (GH-25540)
https://github.com/python/cpython/commit/28575923a9ee40928a9d00a7ed997a6f6a09b8d1
History
Date User Action Args
2021-04-22 23:56:52steve.dowersetmessages: + msg391642
2021-04-22 23:48:50steve.dowersetmessages: + msg391641
2021-04-22 23:34:25miss-islingtonsetpull_requests: + pull_request24266
2021-04-22 23:34:17miss-islingtonsetpull_requests: + pull_request24265
2021-04-22 22:39:17steve.dowersetpull_requests: + pull_request24261
2021-04-22 20:30:44eryksunsetmessages: + msg391632
2021-04-22 20:10:04miss-islingtonsetmessages: + msg391629
2021-04-22 20:07:09miss-islingtonsetmessages: + msg391628
2021-04-22 19:48:08steve.dowersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-04-22 19:45:20miss-islingtonsetpull_requests: + pull_request24250
2021-04-22 19:45:13miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request24249
2021-04-22 19:45:10steve.dowersetmessages: + msg391625
2021-04-22 19:02:29steve.dowersetkeywords: + patch
stage: patch review
pull_requests: + pull_request24246
2021-03-30 18:33:12eryksunsettitle: Inconsistent os.stat behavior for directory with Access Denied -> [Windows] Inconsistent os.stat behavior for directory with Access Denied
components: + Extension Modules
versions: + Python 3.10, - Python 3.7
2021-02-22 06:49:55eryksunsetmessages: + msg387504
2021-02-21 09:24:12CrouZsetmessages: + msg387445
versions: + Python 3.9
2021-02-18 13:13:09nw0setnosy: + nw0
2021-01-09 14:44:44CrouZsetmessages: + msg384729
versions: + Python 3.8
2019-11-18 21:53:30eryksunsetnosy: + eryksun
messages: + msg356915
2019-11-18 17:03:58steve.dowersetmessages: + msg356885
2019-11-16 18:14:54CrouZcreate