[issue22107] tempfile module misinterprets access denied error on Windows

[Billy McCulloch]

Billy McCulloch <bjmcculloch at gmail.com> added the comment:

I stand by the patch file I previously submitted on 2016-05-04. A more detailed analysis / description of my reasoning follows.

Change 1 in _get_default_tempdir:
A PermissionError is thrown on Windows if you attempt to create a file whose filename matches an existing directory. As the code currently stands, the `if` statement checks whether the proposed file's *parent* directory is a directory (which it is, or a FileNotFoundError would have been thrown), instead of whether the proposed filename conflicts with an existing directory. The edited expression is really a typo that, in the context of the code block, always evaluates `True`.
Here’s what we’re now saying in the `if` block: when a PermissionError is raised, if we’re on Windows (the only currently supported platform to throw nonsense errors at us) AND the filename we chose simply conflicts with an existing directory AND we supposedly have write access to the parent directory, then we were just unlucky with the chosen name and should try again in the same parent directory. (I say supposedly because Windows seems to erroneously report True on this check, even when we don’t have write access. I wouldn’t be surprised if this last check does something useful in certain contexts, I just don’t know what they are.)

Change 2 in _mkstemp_inner:
Same as above for Change 1. While _get_default_tempdir uses this code block to make sure the system tempdir is really writable, and _mkstemp_inner does it so that a file descriptor can be returned, the result and arguments are the same.

Change 3 in mkdtemp:
For _get_default_tempdir and _mkstemp_inner, the blocks of code in question are creating temporary files. As such, they need to handle the oddball case for Windows where attempts to create a file with a filename which conflicts with an existing directory name result in a PermissionError. The same block of error handling code is copied in mkdtemp, even though this function never tries to create a file – it only tries to create a directory. As such, in the case that we try to create a directory with a name that already exists (whether as a file or a directory), we wouldn't be dealing with a PermissionError, we'd have a FileExistsError, which is already handled in mkdtemp by the preceding lines. The only way I’ve seen a PermissionError crop up for a call to mkdir on Windows is if the user doesn’t have permission to create filesystem objects in the parent directory. This is the intended usage of a PermissionError, so no special handling needed is required. Remember, a PermissionError shouldn’t happen if mkdtemp is called without a `dir` kwarg, because the _sanitize_params will invoke _get_default_tempdir, which will check to ensure that the parent directory is writable. As such, this block of code was superfluous, and the patch should not raise PermissionError in user code where it previously was caught.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue22107>
_______________________________________