Refactor `nt._path_is*` & `nt._path_[l]exists`

[nineteendo]

Feature or enhancement

Proposal:

Quoting @eryksun:

I agree that all of these _path_is* and _path_[l]exists helpers are a lot of code to maintain, taken together. It could be refactored into smaller inline helper functions that can be reused, which would also make the code more readable.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

• gh-117841: Add C implementation of ntpath.lexists #117842

Linked PRs

• gh-118507 : Refactor nt._path_is* #118755
• [3.13] gh-118507 : Refactor nt._path_is* to improve applicability for other cases (GH-118755) #119360
• [3.12] gh-118507 : Fix ntpath.isfile for pipes. #119381
• [3.12] gh-118507 : Speed up ntpath.isjunction() & ntpath.lexists() with native helpers (GH-118755) #119420
• gh-118507: Amend news entry #120817
• [3.13] gh-118507: Amend news entry to mention ntpath.isfile bugfix (GH-120817) #121365

[nineteendo]

Eryk, do you have any ideas to clean them up (besides the simpler error handling)?

[eryksun]

Here's some code for the _path_is* checks. Using this it would be easy to also implement a _path_isjunction() function for checking mount points.

#define PY_IFREG 1
#define PY_IFDIR 2
#define PY_IFLNK 3
#define PY_IFMNT 4

static BOOL
_testFileTypeByHandle(HANDLE hfile, int refFileType, BOOL diskOnly)
{
    assert(refFileType == PY_IFREG || refFileType == PY_IFDIR ||
           refFileType == PY_IFLNK || refFileType == PY_IFMNT);

    DWORD fileDevType = GetFileType(hfile);
    if ((fileDevType == FILE_TYPE_CHAR) ||
        (fileDevType != FILE_TYPE_DISK && diskOnly) ||
        (fileDevType == FILE_TYPE_UNKNOWN && GetLastError() != 0))
    {
        return FALSE;
    }

    FILE_ATTRIBUTE_TAG_INFO info;
    DWORD attributes, reparseTag;
    if (GetFileInformationByHandleEx(hfile, FileAttributeTagInfo, &info,
                                     sizeof(info)))
    {
        attributes = info.FileAttributes;
        reparseTag = info.ReparseTag;
    }
    else {
        FILE_BASIC_INFO info;
        if (GetFileInformationByHandleEx(hfile, FileBasicInfo, &info,
                                         sizeof(info)))
        {
            attributes = info.FileAttributes;
            reparseTag = 0;
        }
        else {
            return FALSE;
        }
    }

    if (attributes & FILE_ATTRIBUTE_REPARSE_POINT) {
        if (refFileType == PY_IFLNK) {
            return reparseTag == IO_REPARSE_TAG_SYMLINK;
        }
        if (refFileType == PY_IFMNT) {
            return reparseTag == IO_REPARSE_TAG_MOUNT_POINT;
        }
        // Non-surrogate reparse points aren't supported by handle. Just
        // return False. Supporting them requires querying and opening the
        // final path with reparsing enabled.
    }
    else if (refFileType == PY_IFREG) {
        return ((fileDevType == FILE_TYPE_DISK) &&
                !(attributes & FILE_ATTRIBUTE_DIRECTORY));
    }
    else if (refFileType == PY_IFDIR) {
        return attributes & FILE_ATTRIBUTE_DIRECTORY;
    }

    return FALSE;
}

static BOOL
_testFileTypeByName(LPCWSTR path, int refFileType)
{
    assert(refFileType == PY_IFREG || refFileType == PY_IFDIR ||
           refFileType == PY_IFLNK || refFileType == PY_IFMNT);

    FILE_STAT_BASIC_INFORMATION info;
    if (_Py_GetFileInformationByName(path, FileStatBasicByNameInfo,
                                     &info, sizeof(info))) {
        if (info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) {
            if (refFileType == PY_IFLNK) {
                return info.ReparseTag == IO_REPARSE_TAG_SYMLINK;
            }
            else if (refFileType == PY_IFMNT) {
                return info.ReparseTag == IO_REPARSE_TAG_MOUNT_POINT;
            }
            else if ((refFileType == PY_IFREG) &&
                     (info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY))
            {
                return FALSE;
            }
            else if ((refFileType == PY_IFDIR) &&
                     !(info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY))
            {
                return FALSE;
            }
        }
        else if (refFileType == PY_IFLNK || refFileType == PY_IFMNT) {
            return FALSE;
        }
        else if (refFileType == PY_IFREG) {
            return !(info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY);
        }
        else if (refFileType == PY_IFDIR) {
            return info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY;
        }
    }
    else if (_Py_GetFileInformationByName_ErrorIsTrustworthy(
                GetLastError()))
    {
        return FALSE;
    }

    HANDLE hfile;
    DWORD flags = FILE_FLAG_BACKUP_SEMANTICS;
    if (refFileType == PY_IFLNK || refFileType == PY_IFMNT) {
        flags |= FILE_FLAG_OPEN_REPARSE_POINT;
    }
    hfile = CreateFileW(path, FILE_READ_ATTRIBUTES, 0, NULL, OPEN_EXISTING,
                        flags, NULL);
    if (hfile != INVALID_HANDLE_VALUE) {
        BOOL result = _testFileTypeByHandle(hfile, refFileType, FALSE);
        CloseHandle(hfile);
        return result;
    }

    int rc;
    STRUCT_STAT st;
    switch (GetLastError()) {
    case ERROR_ACCESS_DENIED:
    case ERROR_SHARING_VIOLATION:
    case ERROR_CANT_ACCESS_FILE:
    case ERROR_INVALID_PARAMETER:
        if (refFileType == PY_IFREG || refFileType == PY_IFDIR) {
            rc = STAT(path, &st);
        }
        else {
            rc = LSTAT(path, &st);
        }
        if (!rc) {
            switch (refFileType) {
            case PY_IFREG:
                return S_ISREG(st.st_mode);
            case PY_IFDIR:
                return S_ISDIR(st.st_mode);
            case PY_IFLNK:
                return S_ISLNK(st.st_mode);
            case PY_IFMNT:
                return st.st_reparse_tag == IO_REPARSE_TAG_MOUNT_POINT;
            }
        }
    }

    return FALSE;
}


/*[clinic input]
os._path_isdir

    s: 'O'

Return true if the pathname refers to an existing directory.

[clinic start generated code]*/

static PyObject *
os__path_isdir_impl(PyObject *module, PyObject *s)
/*[clinic end generated code: output=9d87ab3c8b8a4e61 input=c17f7ef21d22d64e]*/
{
    path_t _path = PATH_T_INITIALIZE("isdir", "s", 0, 1);
    BOOL result = FALSE;

    if (!path_converter(s, &_path)) {
        path_cleanup(&_path);
        if (PyErr_ExceptionMatches(PyExc_ValueError)) {
            PyErr_Clear();
            Py_RETURN_FALSE;
        }
        return NULL;
    }

    Py_BEGIN_ALLOW_THREADS
    if (_path.fd != -1) {
        HANDLE hfile = _Py_get_osfhandle_noraise(_path.fd);
        if (hfile != INVALID_HANDLE_VALUE) {
            result = _testFileTypeByHandle(hfile, PY_IFDIR, TRUE);
        }
    }
    else if (_path.wide) {
        result = _testFileTypeByName(_path.wide, PY_IFDIR);
    }
    Py_END_ALLOW_THREADS

    path_cleanup(&_path);
    if (result) {
        Py_RETURN_TRUE;
    }
    Py_RETURN_FALSE;
}


/*[clinic input]
os._path_isfile

    path: 'O'

Test whether a path is a regular file

[clinic start generated code]*/

static PyObject *
os__path_isfile_impl(PyObject *module, PyObject *path)
/*[clinic end generated code: output=2394ed7c4b5cfd85 input=de22d74960ade365]*/
{
    path_t _path = PATH_T_INITIALIZE("isfile", "path", 0, 1);
    BOOL result = FALSE;

    if (!path_converter(path, &_path)) {
        path_cleanup(&_path);
        if (PyErr_ExceptionMatches(PyExc_ValueError)) {
            PyErr_Clear();
            Py_RETURN_FALSE;
        }
        return NULL;
    }

    Py_BEGIN_ALLOW_THREADS
    if (_path.fd != -1) {
        HANDLE hfile = _Py_get_osfhandle_noraise(_path.fd);
        if (hfile != INVALID_HANDLE_VALUE) {
            result = _testFileTypeByHandle(hfile, PY_IFREG, TRUE);
        }
    }
    else if (_path.wide) {
        result = _testFileTypeByName(_path.wide, PY_IFREG);
    }
    Py_END_ALLOW_THREADS


    path_cleanup(&_path);
    if (result) {
        Py_RETURN_TRUE;
    }
    Py_RETURN_FALSE;
}


/*[clinic input]
os._path_islink

    path: 'O'

Test whether a path is a symbolic link

[clinic start generated code]*/

static PyObject *
os__path_islink_impl(PyObject *module, PyObject *path)
/*[clinic end generated code: output=6d8640b1a390c054 input=38a3cb937ccf59bf]*/
{
    path_t _path = PATH_T_INITIALIZE("islink", "path", 0, 1);
    BOOL result = FALSE;

    if (!path_converter(path, &_path)) {
        path_cleanup(&_path);
        if (PyErr_ExceptionMatches(PyExc_ValueError)) {
            PyErr_Clear();
            Py_RETURN_FALSE;
        }
        return NULL;
    }

    Py_BEGIN_ALLOW_THREADS
    if (_path.fd != -1) {
        HANDLE hfile = _Py_get_osfhandle_noraise(_path.fd);
        if (hfile != INVALID_HANDLE_VALUE) {
            result = _testFileTypeByHandle(hfile, PY_IFLNK, TRUE);
        }
    }
    else if (_path.wide) {
        result = _testFileTypeByName(_path.wide, PY_IFLNK);
    }
    Py_END_ALLOW_THREADS

    path_cleanup(&_path);
    if (result) {
        Py_RETURN_TRUE;
    }
    Py_RETURN_FALSE;
}

#undef PY_IFREG
#undef PY_IFDIR
#undef PY_IFLNK
#undef PY_IFMNT

[nineteendo]

Good start, we probably need a follow_symlinks option for _testFileTypeByName to use LSTAT() though.

[eryksun]

Sorry, that was an oversight. No new option is needed. I'll edit my post after I rebuild.

[eryksun]

Now it uses LSTAT when checking for a symlink or mount point.

[nineteendo]

Did you speed something up, because it's only 3 lines less than the old code.

[eryksun]

My goal was to remove duplicated code and divide the work into two separate operations that can be understood and modified independently, written in a way that I think is easy to understand. This reduces the maintenance burden. I also added internal support for checking for mount points, in case we implement _path_isjunction().

I added a GetFileType() check in the by-handle code. When combined with the diskOnly parameter, the GetFileType() check makes it safer to check an open file descriptor since the check is implemented directly in the I/O manager without trying to acquire the file lock. If GetFileInformationByHandleEx() is called on a handle for a synchronous-mode pipe, it could block indefinitely. For example, with the current implementation in 3.12:

>>> pr, pw = os.pipe()
>>> threading.Thread(target=os.read, args=(pr, 1)).start()
>>> os.path.isfile(pr)
^C

Also, given the system's named-pipe filesystem (NPFS) support for basic file information (anonymous pipes are also in NPFS, but they're like unlinked open files in POSIX), the GetFileType() check avoids classifying pipes as regular files. Again, with the current implementation in 3.12:

>>> pr, pw = os.pipe()
>>> os.path.isfile(pr)
True

[nineteendo]

OK, that explains why the code isn't much shorter. I added a test for the second issue. The first one seems difficult to test.