os: Users of path_converter don't handle fd == -1 properly

[izbyshev]

BPO	32887
Nosy	@larryhastings, @izbyshev

^{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 2018-02-20.16:24:10.469>
labels = ['type-bug', '3.7']
title = "os: Users of path_converter don't handle fd == -1 properly"
updated_at = <Date 2018-02-20.16:28:20.371>
user = 'https://github.com/izbyshev'

bugs.python.org fields:

activity = <Date 2018-02-20.16:28:20.371>
actor = 'izbyshev'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2018-02-20.16:24:10.469>
creator = 'izbyshev'
dependencies = []
files = []
hgrepos = []
issue_num = 32887
keywords = []
message_count = 1.0
messages = ['312421']
nosy_count = 2.0
nosy_names = ['larry', 'izbyshev']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32887'
versions = ['Python 3.6', 'Python 3.7']

[izbyshev]

Demo:
>>> import os
>>> os.chdir(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 14] Bad address: -1
>>> os.chdir(-2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 9] Bad file descriptor: -2

Functions in os supporting either path or file descriptor argument (os.supports_fd) usually use the following code pattern to distinguish between those cases:

if (path->fd != -1)
    result = fchdir(path->fd);
else
    result = chdir(path->narrow);

However, _fd_converter used by path_converter internally doesn't give any special meaning to -1 and allows any negative file descriptors. Therefore, if a user passes -1 to such function, path->narrow, which is NULL, will be used.

I see two ways to fix this.

1. Make some flag in path_t indicating that it should be treated as fd and make all users check that flag.
2. Make _fd_converter raise an exception for negative descriptors.

Also, I have to mention an inconsistency in reporting of bad descriptors. A handful of os functions uses fildes_converter for descriptors, which uses PyObject_AsFileDescriptor, which in turn is used in other places in Python as well (e.g. in fcntl module). PyObject_AsFileDescriptor raises a ValueError for negative descriptors instead of OSError raised by most os functions in this case.

>>> os.fchdir(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: file descriptor cannot be a negative integer (-1)