classification
Title: Type: Improper NotADirectoryError when opening a file in a fake directory behavior patch review IO Python 3.8
process
Status: Resolution: open danny87105, eryksun, iritkatriel normal patch

Created on 2020-09-07 15:36 by danny87105, last changed 2020-10-20 18:18 by eryksun.

Pull Requests
PR 22818 open iritkatriel, 2020-10-20 12:46
Messages (10)
msg376505 - (view) Author: Danny Lin (danny87105) Date: 2020-09-07 15:36
On Linux (tested on Ubuntu 16.04), if "/path/to/file" is an existing file, the code:

open('/path/to/file/somename.txt')

raises NotADirectoryError: [Errno 20] Not a directory: '/path/to/file/somename.txt'

On Windows, similar code:

open(r'C:\path\to\file\somename.txt')

raises FileNotFoundError: [Errno 2] No such file or directory: 'C:\\path\\chrome\\to\\file\\somename.txt'

I think the behavior on Linux is not correct. The user probably cares about the existence of the file to be opened, rather than whether its ancestor directories are valid.

OTOH, if NotADirectoryError should be raised, it should mention '/path/to/file' rather then '/path/to/file/somename.txt'. But what if '/path/to' or '/path' is actually a file? Should it be '/path/to' or '/path' instead for the same reason?
msg376529 - (view) Author: Eryk Sun (eryksun) * Date: 2020-09-07 21:14
> if NotADirectoryError should be raised, it should mention '/path/to/file'
> rather then '/path/to/file/somename.txt'.

POSIX specifies that C open() should set errno to ENOTDIR when an existing path prefix component is neither a directory nor a symlink to a directory [1]. What you propose isn't possible to implement reliably unless the filesystem is locked or readonly, so it should be handled by the application instead of by the system or standard libraries.

> FileNotFoundError: [Errno 2] No such file or directory:
> 'C:\\path\\chrome\\to\\file\\somename.txt'

Windows specifies that this case should fail as follows: "[i]f Link.File.FileType is not DirectoryFile, the operation MUST be failed with STATUS_OBJECT_PATH_NOT_FOUND" [2] (see Phase 6 in the linked pseudocode).

The Windows API maps this status code to ERROR_PATH_NOT_FOUND (3), which is distinct from ERROR_FILE_NOT_FOUND (2). However, the C runtime maps both of these system error codes to POSIX ENOENT. All isn't lost, however, because it also saves the OS error in _doserrno. io.FileIO could preset _doserrno to 0, and if it's non-zero after calling _wopen, use its value with PyErr_SetExcFromWindowsErrWithFilenameObject instead of calling PyErr_SetFromErrnoWithFilenameObject.

---

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
[2] https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/8ada5fbe-db4e-49fd-aef6-20d54b748e40
msg376562 - (view) Author: Danny Lin (danny87105) Date: 2020-09-08 10:24
I'm not so familiar about the spec. If such behavior is confirmed due to implementation difference across OSes, and it's also not desirable to change the mapping of the OS error to Python exception, we can simplify left it as-is.

However, this behavior difference can potentially cause cross-platform compatibility. For example, the code:

try:
open('/path/to/file/somename.txt')
except FileNotFoundError:
"""do something"""

would work on Windows but break on Linux (or POSIX?).

The current (3.8) description for exception NotADirectoryError is:

Raised when a directory operation (such as os.listdir()) is requested on something which is not a directory. Corresponds to errno ENOTDIR.

According to this, a user probably won't expect to receive a NotADirectoryError for open('/path/to/file/somename.txt'), as this doesn't seem like a directory operation at all, unless he is expert enough to know about the implication of errno ENOTDIR.

I think a note should at least be added to the documentation if we are not going to change the behavior.
msg377288 - (view) Author: Irit Katriel (iritkatriel) * Date: 2020-09-21 22:02
The documentation for open says "If the file cannot be opened, an OSError is raised."

NotADirectoryError and FileNotFoundError are both OSErrors.

So the correct way to write Danny's code snippet, according to the documentation, is:

try:
open('/path/to/file/somename.txt')
except OSError:
"""do something"""
msg379125 - (view) Author: Danny Lin (danny87105) Date: 2020-10-20 13:54
By writing "except FileNotFoundError:", the intention is to catch an error when the file being opened is not found, and don't catch an error for other cases, such as an existing file without adequate permission. Writing "except OSError:" catches just too much cases including unwanted ones.

As they are not equivalent, you cannot say that the latter is the "correct" way for the former.

And you totally omitted the argument for the inadequate documentation for NotADirectoryError. It says NotADirectoryError is raises when a DIRECTORY operation is requested, and does not cover the case of opening a file.
msg379128 - (view) Author: Irit Katriel (iritkatriel) * Date: 2020-10-20 14:23
Hi Danny,

I'm not saying that OSError and FileNotFoundError are equivalent. I'm saying that the open() API, as documented, raises OSError when opening the file fails.

The way to check whether a file exists is to use os.path.exists(path) or os.path.isfile(path).

I don't quite follow your last point - opening a file is indeed a file operation, but it contains within it a directory operation (finding the file).
msg379131 - (view) Author: Danny Lin (danny87105) Date: 2020-10-20 14:36
I don't think a general developer would expect that open('/path/to/file/somename.txt') implies a directory operation, and it also doesn't on Windows.

I suggest that a further notice be added to NotADirectoryError, such as:

Raised when a directory operation (such as os.listdir()) is requested on something which is not a directory. Corresponds to errno ENOTDIR. In some filesystem such as POSIX, NotADirectoryError (ENOTDIR) is raised when attempting to open a path whose ancestor is not a directory.
msg379136 - (view) Author: Irit Katriel (iritkatriel) * Date: 2020-10-20 14:53
> I don't think a general developer would expect that open('/path/to/file/somename.txt') implies a directory operation, and it also doesn't on Windows.

Really? It's not obvious that finding a file would involve directory operations?

In what sense does it even matter whether you expect a directory operation to happen? The contract is that if the open() fails you get an OSError. The documentation doesn't say which OSError, and that is in fact a platform-specific implementation detail.
msg379141 - (view) Author: Danny Lin (danny87105) Date: 2020-10-20 15:26
> Really? It's not obvious that finding a file would involve directory operations?

Not in some senses, and that's possibly why Windows does not raise NotADirectoryError in such case.

I agree that it's a platform-specific implementation detail. However, there are lots of platform-specific implementation details written in documentation elsewhere already, especially OS related modules such as os and os.path. I don't think mentioning platform-specific implementation details for subclasses of OSError would be any less reasonable than that.

Adding such notice for NotADirectoryError helps people who want a EAFP style code for nonexistent file by preventing them getting trapped writing a cross-platform code.
msg379150 - (view) Author: Eryk Sun (eryksun) * Date: 2020-10-20 18:18
Regarding documentation, builtin open() and the io and os modules generally do not provide information about platform-specific errors. But documenting the behavior for the NotADirectoryError exception itself may be useful considering it applies to many POSIX functions that access filepaths, such as stat, open, mkdir, rmdir, unlink, and rename.

Regarding behavior, I don't see anything reasonable that can or should be done for POSIX. In Windows, it should be possible to know whether FileNotFoundError is due to a bad path prefix (ERROR_PATH_NOT_FOUND, 3) or a missing file (ERROR_FILE_NOT_FOUND, 2), but io.FileIO currently only uses standard C errno, which maps both of these cases to ENOENT. So one behavior that *can* be fixed in this situation is to get the actual Windows error code from MSVC _doserrno, such that the raised FileNotFoundError would have the relevant Windows error code in its winerror attribute.
History
Date User Action Args
2020-10-20 18:18:14eryksunsetmessages: + msg379150
2020-10-20 15:26:54danny87105setmessages: + msg379141
2020-10-20 14:53:27iritkatrielsetmessages: + msg379136
2020-10-20 14:36:13danny87105setmessages: + msg379131
2020-10-20 14:23:09iritkatrielsetmessages: + msg379128
2020-10-20 13:54:41danny87105setmessages: + msg379125
2020-10-20 12:46:47iritkatrielsetkeywords: + patch
stage: patch review
pull_requests: + pull_request21774
2020-09-21 22:02:39iritkatrielsetnosy: + iritkatriel
messages: + msg377288
components: + IO
2020-09-11 23:23:46terry.reedysettitle: Improper NotADirectoryError when opening a file under a fake directory -> Improper NotADirectoryError when opening a file in a fake directory
2020-09-08 10:24:50danny87105setmessages: + msg376562
2020-09-07 21:14:39eryksunsetnosy: + eryksun
messages: + msg376529
2020-09-07 15:36:37danny87105create