Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os.path.exists() takes bool as argument and returns True #82626

Closed
Nika mannequin opened this issue Oct 11, 2019 · 8 comments
Closed

os.path.exists() takes bool as argument and returns True #82626

Nika mannequin opened this issue Oct 11, 2019 · 8 comments
Assignees
Labels
3.9 only security fixes 3.10 only security fixes topic-IO type-bug An unexpected behavior, bug, or error

Comments

@Nika
Copy link
Mannequin

Nika mannequin commented Oct 11, 2019

BPO 38445
Nosy @serhiy-storchaka, @eryksun, @rmariano

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 = 'https://github.com/serhiy-storchaka'
closed_at = None
created_at = <Date 2019-10-11.11:16:42.144>
labels = ['3.10', 'type-bug', '3.9', 'expert-IO']
title = 'os.path.exists() takes bool as argument and returns True'
updated_at = <Date 2020-09-22.06:20:54.283>
user = 'https://bugs.python.org/Nika'

bugs.python.org fields:

activity = <Date 2020-09-22.06:20:54.283>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = False
closed_date = None
closer = None
components = ['IO']
creation = <Date 2019-10-11.11:16:42.144>
creator = 'Nika'
dependencies = []
files = []
hgrepos = []
issue_num = 38445
keywords = []
message_count = 8.0
messages = ['354440', '354443', '354444', '354481', '354495', '354510', '354514', '377303']
nosy_count = 4.0
nosy_names = ['serhiy.storchaka', 'eryksun', 'Mariano Anaya', 'Nika']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38445'
versions = ['Python 3.9', 'Python 3.10']

Linked PRs

@Nika
Copy link
Mannequin Author

Nika mannequin commented Oct 11, 2019

os.path.exists() accepts either True or False as argument and returns True.
Reproducible on Windows, e. g., in jupyter notebook or in Eclipse, although not in IDLE, which returns False, as expected.

import os
print(os.path.exists(False))

@Nika Nika mannequin added 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Oct 11, 2019
@rmariano
Copy link
Mannequin

rmariano mannequin commented Oct 11, 2019

Hi, I would like to take a look at this issue. Could someone please assign it to me? Thanks

@eryksun
Copy link
Contributor

eryksun commented Oct 11, 2019

Note that the underlying stat call supports file descriptors, which are non-negative integers. This is a supported and tested capability for genericpath.exists (see GenericTest.test_exists_fd in Lib/test/test_genericpath.py).

False and True are integers with the values 0 and 1:

    >>> issubclass(bool, int)
    True
    >>> False + 0
    0
    >>> True + 0
    1

That can be useful, but there may be cases where we don't want to conflate bools and integers. IMO, a bool should not be supported as a file descriptor. It's likely a bug that should be caught early instead of meaninglessly propagated.

A high-level solution would check for bool instances in genericpath.exists. A low-level solution, to make this policy consistent in general, would be to modify _fd_converter in Modules/posixmodule.c to disallow bool instances.

@serhiy-storchaka
Copy link
Member

Eryk, I think testing for bool in _fd_converter and issuing a warning is a good idea. Do you want to create a PR?

@vstinner
Copy link
Member

Maybe os.fspath() can be used:

>>> os.fspath(True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: expected str, bytes or os.PathLike object, not bool

@taleinat
Copy link
Contributor

Mariano Anaya, you're welcome to work on this and create a PR.

There's no need to have this issue assigned to you: feel free to just begin working. Anyone reading this will know that you intended to work on this.

(FYI, we only assign issues to core developers, and assigning means something slightly different than "who is working on this.")

@eryksun
Copy link
Contributor

eryksun commented Oct 12, 2019

Maybe os.fspath() can be used

I think that would already be the case if genericpath.exists didn't have to support file descriptors. It's documented that the path argument "refers to an existing path or an open file descriptor".

Modifying _fd_converter would provide consistent behavior for all calls that ultimately use argument clinic's path_t(allow_fd=True) and/or dir_fd. Serhiy's suggestion to raise a warning sounds good.

Here are some examples that currently pass silently in Linux:

>>> os.chown(False, 1000, 1000)
>>> os.chmod(False, 0o666)
>>> os.utime(False, (1500000000, 1500000000))

Probably os.fstat and other "f" functions (e.g. fstatvfs, fchdir, fchown, fchmod, ftruncate, fdatasync, fsync, and fpathconf) should also raise a warning when passed a bool. For example, the following would raise a warning instead of passing silently:

    >>> os.fstat(False).st_size
    0

These cases could be addressed by consistently using an argument clinic type. Some of them already us the fildes type (e.g. fchdir, fsync, fdatasync). However, fildes_converter calls PyObject_AsFileDescriptor, which also supports objects with a fileno() method. That's documented behavior for PyObject_AsFileDescriptor, but nothing in the documentation of fchdir, fsync, and fdatasync suggests to me that they support objects with a fileno() method:

    >>> os.fchdir(sys.stdin)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    NotADirectoryError: [Errno 20] Not a directory

If not for this behavior, we could simply change all of the "f" functions to use the fildes type.

PyObject_AsFileDescriptor is used in various other places as well, such as the select module. In the latter case, supporting objects with a fileno() method is clearly documented.

Also consider including open() and os.fdopen by modifying _io_open_impl in Modules/_io/_iomodule.c. For example:

   >>> open(False, closefd=False)
    <_io.TextIOWrapper name=False mode='r' encoding='UTF-8'>

Currently it calls PyNumber_Check(file). If true, a warning could be raised if PyBool_Check(file) is also true.

@iritkatriel iritkatriel added topic-IO 3.9 only security fixes 3.10 only security fixes and removed 3.7 (EOL) end of life labels Sep 21, 2020
@serhiy-storchaka
Copy link
Member

It is a large change, so I take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes 3.10 only security fixes topic-IO type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants