tempfile module misinterprets access denied error on Windows

[rupole]

BPO	22107
Nosy	@pfmoore, @ncoghlan, @tjguk, @bobince, @takluyver, @zware, @serhiy-storchaka, @eryksun, @zooba, @RoccoMatano, @earonesty
Files	tempfile_bad_tempdir.patch tempfile_bad_tempdir_2.patch tempfile_bad_tempdir_3.patch tempfile_bad_tempdir_4.patch master...bjmcculloch_patch-1.diff: fix PermissionError exception handlers

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2014-07-30.12:40:41.492>
labels = ['3.8', '3.9', '3.10', 'performance', 'library', 'OS-windows']
title = 'tempfile module misinterprets access denied error on Windows'
updated_at = <Date 2021-07-08.18:35:20.084>
user = 'https://bugs.python.org/rupole'

bugs.python.org fields:

activity = <Date 2021-07-08.18:35:20.084>
actor = 'bugale bugale'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2014-07-30.12:40:41.492>
creator = 'rupole'
dependencies = []
files = ['38145', '38151', '39421', '39434', '42704']
hgrepos = []
issue_num = 22107
keywords = ['patch']
message_count = 43.0
messages = ['224304', '236004', '236028', '236029', '236047', '236050', '236052', '236053', '236055', '236058', '236060', '236112', '236118', '236130', '236132', '236133', '236143', '243514', '243608', '243611', '243617', '243626', '243628', '243787', '243788', '243805', '243808', '243812', '243814', '243815', '259559', '264779', '264971', '265040', '282403', '285131', '337735', '340738', '347076', '347095', '353353', '353360', '357075']
nosy_count = 20.0
nosy_names = ['paul.moore', 'ncoghlan', 'rupole', 'tim.golden', 'aclover', 'python-dev', 'takluyver', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'Tor.Colvin', 'rocco.matano', 'V\xc3\xa1clav Dvo\xc5\x99\xc3\xa1k', 'Billy McCulloch', 'Paul Doom', 'earonesty', 'bugale bugale', 'JDM', 'Jonathan Mills']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue22107'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[rupole]

_mkstemp_inner assumes that an access denied error means that it
has generated a filename that matches an existing foldername.
However, in the case of a folder for which you don't have permissions to
create a file, this means it will loop thru the maximum possible number of files.
This causes it to hang for several seconds and eventually return a bogus
FileExistsError.

Similar behaviour exists in 2.7.7, but it throws an IOError instead.

http://bugs.python.org/issue18849 seems to be where this was introduced.

[BreamoreBoy]

changeset 035b61b52caa has this:-

             return (fd, _os.path.abspath(file))
         except FileExistsError:
             continue    # try again
+        except PermissionError:
+            # This exception is thrown when a directory with the chosen name
+            # already exists on windows.
+            if _os.name == 'nt':
+                continue
+            else:
+                raise
 
     raise FileExistsError(_errno.EEXIST,
                           "No usable temporary file name found")

Could we simply set a flag saying it's a PermissionError and then raise the appropriate PermissionError or FileExistsError at the end of the loop?

[serhiy-storchaka]

What exceptions are raised on Windows when try to open a file in bad directory?

open('foo').close()
open('foo/bar') # what raised?
open('nonexistent/bar') # what raised?

If raised the same exceptions as on Linux, then perhaps the following patch fixes this issue.

[serhiy-storchaka]

And what returns os.access for writable directories and non-existent files?

os.mkdir('dir')
os.access('dir', os.W_OK)          # what returns?
os.access('nonexistent', os.W_OK)  # what returns or raises?

[BreamoreBoy]

>>> os.mkdir('dir')
>>> os.access('dir', os.W_OK)
True
>>> os.access('nonexistent', os.W_OK)
False
>>> open('dir/bar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'dir/bar'
>>> open('nonexistent/bar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'nonexistent/bar'

[serhiy-storchaka]

Thank you Mark. Could you please make first test in msg236028 (when first part of the path is a file, not a directory)?

[BreamoreBoy]

>>> open('README').close()
>>> open('README/bar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'README/bar'

[zooba]

Is there a difference if you do open(..., 'w')? It's a different enough operation that it may have a different error.

[serhiy-storchaka]

Is there a difference if you do open(..., 'w')? It's a different enough operation that it may have a different error.

Oh, yes, I forgot the 'w' mode.

Mark, could you please run following test on Windows?

import os
open('foo', 'wb').close()
flags = os.O_RDWR | os.O_CREAT | os.O_EXCL | getattr(os, 'O_NOFOLLOW', 0) | getattr(os, 'O_BINARY', 0)
os.open('foo/bar', flags, 0o600)          # what raised?
os.open('nonexistent/bar', flags, 0o600)  # what raised?

[BreamoreBoy]

>>> open('foo', 'wb').close()
>>> flags = os.O_RDWR | os.O_CREAT | os.O_EXCL | getattr(os, 'O_NOFOLLOW', 0) | getattr(os, 'O_BINARY', 0)
>>> os.open('foo/bar', flags, 0o600)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'foo/bar'
>>> os.open('nonexistent/bar', flags, 0o600)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'nonexistent/bar'

[serhiy-storchaka]

Great. There is only one difference between Windows and Linux, but it affects only error type in tests. Here is a patch with updated test. It should now work on Windows.

[rupole]

os.access doesn't check filesystem permissions, so the patch will not catch the condition that creates the problem.

[serhiy-storchaka]

It is best that we can do. How else we can check filesystem permissions? Only trying to create a file, But we just tried this and it failed.