bpo-30555: Fix WindowsConsoleIO fails in the presence of fd redirection (GH-1927)

This works by not caching the handle and instead getting the handle from
the file descriptor each time, so that if the actual handle changes by
fd redirection closing/opening the console handle beneath our feet, we
will keep working correctly.
This commit is contained in:
Segev Finer 2021-04-24 01:00:27 +03:00 committed by GitHub
parent 6b59e662fa
commit 5e437fb872
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 144 additions and 125 deletions

View File

@ -161,4 +161,12 @@ PyAPI_FUNC(int) _Py_dup(int fd);
PyAPI_FUNC(int) _Py_get_blocking(int fd);
PyAPI_FUNC(int) _Py_set_blocking(int fd, int blocking);
#endif /* !MS_WINDOWS */
#else /* MS_WINDOWS */
PyAPI_FUNC(void*) _Py_get_osfhandle_noraise(int fd);
PyAPI_FUNC(void*) _Py_get_osfhandle(int fd);
PyAPI_FUNC(int) _Py_open_osfhandle_noraise(void *handle, int flags);
PyAPI_FUNC(int) _Py_open_osfhandle(void *handle, int flags);
#endif /* MS_WINDOWS */

View File

@ -0,0 +1,2 @@
Fix ``WindowsConsoleIO`` errors in the presence of fd redirection. Patch by
Segev Finer.

View File

@ -8,10 +8,10 @@ PyDoc_STRVAR(_io__WindowsConsoleIO_close__doc__,
"close($self, /)\n"
"--\n"
"\n"
"Close the handle.\n"
"Close the console object.\n"
"\n"
"A closed handle cannot be used for further I/O operations. close() may be\n"
"called more than once without error.");
"A closed console object cannot be used for further I/O operations.\n"
"close() may be called more than once without error.");
#define _IO__WINDOWSCONSOLEIO_CLOSE_METHODDEF \
{"close", (PyCFunction)_io__WindowsConsoleIO_close, METH_NOARGS, _io__WindowsConsoleIO_close__doc__},
@ -110,10 +110,7 @@ PyDoc_STRVAR(_io__WindowsConsoleIO_fileno__doc__,
"fileno($self, /)\n"
"--\n"
"\n"
"Return the underlying file descriptor (an integer).\n"
"\n"
"fileno is only set when a file descriptor is used to open\n"
"one of the standard streams.");
"Return the underlying file descriptor (an integer).");
#define _IO__WINDOWSCONSOLEIO_FILENO_METHODDEF \
{"fileno", (PyCFunction)_io__WindowsConsoleIO_fileno, METH_NOARGS, _io__WindowsConsoleIO_fileno__doc__},
@ -381,4 +378,4 @@ _io__WindowsConsoleIO_isatty(winconsoleio *self, PyObject *Py_UNUSED(ignored))
#ifndef _IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF
#define _IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF
#endif /* !defined(_IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF) */
/*[clinic end generated code: output=a28b3120fa53b256 input=a9049054013a1b77]*/
/*[clinic end generated code: output=48080572ffee22f5 input=a9049054013a1b77]*/

View File

@ -64,10 +64,7 @@ char _PyIO_get_console_type(PyObject *path_or_fd) {
int fd = PyLong_AsLong(path_or_fd);
PyErr_Clear();
if (fd >= 0) {
HANDLE handle;
_Py_BEGIN_SUPPRESS_IPH
handle = (HANDLE)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
HANDLE handle = _Py_get_osfhandle_noraise(fd);
if (handle == INVALID_HANDLE_VALUE)
return '\0';
return _get_console_type(handle);
@ -143,12 +140,11 @@ class _io._WindowsConsoleIO "winconsoleio *" "&PyWindowsConsoleIO_Type"
typedef struct {
PyObject_HEAD
HANDLE handle;
int fd;
unsigned int created : 1;
unsigned int readable : 1;
unsigned int writable : 1;
unsigned int closehandle : 1;
unsigned int closefd : 1;
char finalizing;
unsigned int blksize;
PyObject *weakreflist;
@ -164,7 +160,7 @@ _Py_IDENTIFIER(name);
int
_PyWindowsConsoleIO_closed(PyObject *self)
{
return ((winconsoleio *)self)->handle == INVALID_HANDLE_VALUE;
return ((winconsoleio *)self)->fd == -1;
}
@ -172,16 +168,12 @@ _PyWindowsConsoleIO_closed(PyObject *self)
static int
internal_close(winconsoleio *self)
{
if (self->handle != INVALID_HANDLE_VALUE) {
if (self->closehandle) {
if (self->fd >= 0) {
if (self->fd != -1) {
if (self->closefd) {
_Py_BEGIN_SUPPRESS_IPH
close(self->fd);
_Py_END_SUPPRESS_IPH
}
CloseHandle(self->handle);
}
self->handle = INVALID_HANDLE_VALUE;
self->fd = -1;
}
return 0;
@ -190,15 +182,15 @@ internal_close(winconsoleio *self)
/*[clinic input]
_io._WindowsConsoleIO.close
Close the handle.
Close the console object.
A closed handle cannot be used for further I/O operations. close() may be
called more than once without error.
A closed console object cannot be used for further I/O operations.
close() may be called more than once without error.
[clinic start generated code]*/
static PyObject *
_io__WindowsConsoleIO_close_impl(winconsoleio *self)
/*[clinic end generated code: output=27ef95b66c29057b input=185617e349ae4c7b]*/
/*[clinic end generated code: output=27ef95b66c29057b input=68c4e5754f8136c2]*/
{
PyObject *res;
PyObject *exc, *val, *tb;
@ -206,8 +198,8 @@ _io__WindowsConsoleIO_close_impl(winconsoleio *self)
_Py_IDENTIFIER(close);
res = _PyObject_CallMethodIdOneArg((PyObject*)&PyRawIOBase_Type,
&PyId_close, (PyObject*)self);
if (!self->closehandle) {
self->handle = INVALID_HANDLE_VALUE;
if (!self->closefd) {
self->fd = -1;
return res;
}
if (res == NULL)
@ -229,12 +221,11 @@ winconsoleio_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
self = (winconsoleio *) type->tp_alloc(type, 0);
if (self != NULL) {
self->handle = INVALID_HANDLE_VALUE;
self->fd = -1;
self->created = 0;
self->readable = 0;
self->writable = 0;
self->closehandle = 0;
self->closefd = 0;
self->blksize = 0;
self->weakreflist = NULL;
}
@ -269,16 +260,17 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
int rwa = 0;
int fd = -1;
int fd_is_own = 0;
HANDLE handle = NULL;
assert(PyWindowsConsoleIO_Check(self));
if (self->handle >= 0) {
if (self->closehandle) {
if (self->fd >= 0) {
if (self->closefd) {
/* Have to close the existing file first. */
if (internal_close(self) < 0)
return -1;
}
else
self->handle = INVALID_HANDLE_VALUE;
self->fd = -1;
}
fd = _PyLong_AsInt(nameobj);
@ -341,14 +333,12 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
goto bad_mode;
if (fd >= 0) {
_Py_BEGIN_SUPPRESS_IPH
self->handle = (HANDLE)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
self->closehandle = 0;
handle = _Py_get_osfhandle_noraise(fd);
self->closefd = 0;
} else {
DWORD access = GENERIC_READ;
self->closehandle = 1;
self->closefd = 1;
if (!closefd) {
PyErr_SetString(PyExc_ValueError,
"Cannot use closefd=False with file name");
@ -363,21 +353,31 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj,
on the specific access. This is required for modern names
CONIN$ and CONOUT$, which allow reading/writing state as
well as reading/writing content. */
self->handle = CreateFileW(name, GENERIC_READ | GENERIC_WRITE,
handle = CreateFileW(name, GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL);
if (self->handle == INVALID_HANDLE_VALUE)
self->handle = CreateFileW(name, access,
if (handle == INVALID_HANDLE_VALUE)
handle = CreateFileW(name, access,
FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL);
Py_END_ALLOW_THREADS
if (self->handle == INVALID_HANDLE_VALUE) {
if (handle == INVALID_HANDLE_VALUE) {
PyErr_SetExcFromWindowsErrWithFilenameObject(PyExc_OSError, GetLastError(), nameobj);
goto error;
}
if (self->writable)
self->fd = _Py_open_osfhandle_noraise(handle, _O_WRONLY | _O_BINARY);
else
self->fd = _Py_open_osfhandle_noraise(handle, _O_RDONLY | _O_BINARY);
if (self->fd < 0) {
CloseHandle(handle);
PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
goto error;
}
}
if (console_type == '\0')
console_type = _get_console_type(self->handle);
console_type = _get_console_type(handle);
if (self->writable && console_type != 'w') {
PyErr_SetString(PyExc_ValueError,
@ -460,25 +460,14 @@ _io._WindowsConsoleIO.fileno
Return the underlying file descriptor (an integer).
fileno is only set when a file descriptor is used to open
one of the standard streams.
[clinic start generated code]*/
static PyObject *
_io__WindowsConsoleIO_fileno_impl(winconsoleio *self)
/*[clinic end generated code: output=006fa74ce3b5cfbf input=079adc330ddaabe6]*/
/*[clinic end generated code: output=006fa74ce3b5cfbf input=845c47ebbc3a2f67]*/
{
if (self->fd < 0 && self->handle != INVALID_HANDLE_VALUE) {
_Py_BEGIN_SUPPRESS_IPH
if (self->writable)
self->fd = _open_osfhandle((intptr_t)self->handle, _O_WRONLY | _O_BINARY);
else
self->fd = _open_osfhandle((intptr_t)self->handle, _O_RDONLY | _O_BINARY);
_Py_END_SUPPRESS_IPH
}
if (self->fd < 0)
return err_mode("fileno");
return err_closed();
return PyLong_FromLong(self->fd);
}
@ -492,7 +481,7 @@ static PyObject *
_io__WindowsConsoleIO_readable_impl(winconsoleio *self)
/*[clinic end generated code: output=daf9cef2743becf0 input=6be9defb5302daae]*/
{
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return err_closed();
return PyBool_FromLong((long) self->readable);
}
@ -507,7 +496,7 @@ static PyObject *
_io__WindowsConsoleIO_writable_impl(winconsoleio *self)
/*[clinic end generated code: output=e0a2ad7eae5abf67 input=cefbd8abc24df6a0]*/
{
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return err_closed();
return PyBool_FromLong((long) self->writable);
}
@ -642,7 +631,7 @@ error:
static Py_ssize_t
readinto(winconsoleio *self, char *buf, Py_ssize_t len)
{
if (self->handle == INVALID_HANDLE_VALUE) {
if (self->fd == -1) {
err_closed();
return -1;
}
@ -657,6 +646,10 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len)
return -1;
}
HANDLE handle = _Py_get_osfhandle(self->fd);
if (handle == INVALID_HANDLE_VALUE)
return -1;
/* Each character may take up to 4 bytes in the final buffer.
This is highly conservative, but necessary to avoid
failure for any given Unicode input (e.g. \U0010ffff).
@ -678,7 +671,7 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len)
return read_len;
DWORD n;
wchar_t *wbuf = read_console_w(self->handle, wlen, &n);
wchar_t *wbuf = read_console_w(handle, wlen, &n);
if (wbuf == NULL)
return -1;
if (n == 0) {
@ -784,10 +777,15 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self)
DWORD bufsize, n, len = 0;
PyObject *bytes;
DWORD bytes_size, rn;
HANDLE handle;
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return err_closed();
handle = _Py_get_osfhandle(self->fd);
if (handle == INVALID_HANDLE_VALUE)
return NULL;
bufsize = BUFSIZ;
buf = (wchar_t*)PyMem_Malloc((bufsize + 1) * sizeof(wchar_t));
@ -819,7 +817,7 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self)
buf = tmp;
}
subbuf = read_console_w(self->handle, bufsize - len, &n);
subbuf = read_console_w(handle, bufsize - len, &n);
if (subbuf == NULL) {
PyMem_Free(buf);
@ -909,7 +907,7 @@ _io__WindowsConsoleIO_read_impl(winconsoleio *self, Py_ssize_t size)
PyObject *bytes;
Py_ssize_t bytes_size;
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return err_closed();
if (!self->readable)
return err_mode("reading");
@ -959,12 +957,17 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b)
BOOL res = TRUE;
wchar_t *wbuf;
DWORD len, wlen, n = 0;
HANDLE handle;
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return err_closed();
if (!self->writable)
return err_mode("writing");
handle = _Py_get_osfhandle(self->fd);
if (handle == INVALID_HANDLE_VALUE)
return NULL;
if (!b->len) {
return PyLong_FromLong(0);
}
@ -995,7 +998,7 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b)
Py_BEGIN_ALLOW_THREADS
wlen = MultiByteToWideChar(CP_UTF8, 0, b->buf, len, wbuf, wlen);
if (wlen) {
res = WriteConsoleW(self->handle, wbuf, wlen, &n, NULL);
res = WriteConsoleW(handle, wbuf, wlen, &n, NULL);
if (res && n < wlen) {
/* Wrote fewer characters than expected, which means our
* len value may be wrong. So recalculate it from the
@ -1027,15 +1030,15 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b)
static PyObject *
winconsoleio_repr(winconsoleio *self)
{
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return PyUnicode_FromFormat("<_io._WindowsConsoleIO [closed]>");
if (self->readable)
return PyUnicode_FromFormat("<_io._WindowsConsoleIO mode='rb' closefd=%s>",
self->closehandle ? "True" : "False");
self->closefd ? "True" : "False");
if (self->writable)
return PyUnicode_FromFormat("<_io._WindowsConsoleIO mode='wb' closefd=%s>",
self->closehandle ? "True" : "False");
self->closefd ? "True" : "False");
PyErr_SetString(PyExc_SystemError, "_WindowsConsoleIO has invalid mode");
return NULL;
@ -1051,7 +1054,7 @@ static PyObject *
_io__WindowsConsoleIO_isatty_impl(winconsoleio *self)
/*[clinic end generated code: output=9eac09d287c11bd7 input=9b91591dbe356f86]*/
{
if (self->handle == INVALID_HANDLE_VALUE)
if (self->fd == -1)
return err_closed();
Py_RETURN_TRUE;
@ -1077,13 +1080,13 @@ static PyMethodDef winconsoleio_methods[] = {
static PyObject *
get_closed(winconsoleio *self, void *closure)
{
return PyBool_FromLong((long)(self->handle == INVALID_HANDLE_VALUE));
return PyBool_FromLong((long)(self->fd == -1));
}
static PyObject *
get_closefd(winconsoleio *self, void *closure)
{
return PyBool_FromLong((long)(self->closehandle));
return PyBool_FromLong((long)(self->closefd));
}
static PyObject *

View File

@ -1370,13 +1370,10 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
*/
if (fileno != -1 && fileno != 0) {
/* Ensure that fileno is within the CRT's valid range */
_Py_BEGIN_SUPPRESS_IPH
fh = (HANDLE)_get_osfhandle(fileno);
_Py_END_SUPPRESS_IPH
if (fh==(HANDLE)-1) {
PyErr_SetFromErrno(PyExc_OSError);
fh = _Py_get_osfhandle(fileno);
if (fh == INVALID_HANDLE_VALUE)
return NULL;
}
/* Win9x appears to need us seeked to zero */
lseek(fileno, 0, SEEK_SET);
}

View File

@ -10098,18 +10098,16 @@ os_pipe_impl(PyObject *module)
attr.bInheritHandle = FALSE;
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_SUPPRESS_IPH
ok = CreatePipe(&read, &write, &attr, 0);
if (ok) {
fds[0] = _open_osfhandle((intptr_t)read, _O_RDONLY);
fds[1] = _open_osfhandle((intptr_t)write, _O_WRONLY);
fds[0] = _Py_open_osfhandle_noraise(read, _O_RDONLY);
fds[1] = _Py_open_osfhandle_noraise(write, _O_WRONLY);
if (fds[0] == -1 || fds[1] == -1) {
CloseHandle(read);
CloseHandle(write);
ok = 0;
}
}
_Py_END_SUPPRESS_IPH
Py_END_ALLOW_THREADS
if (!ok)

View File

@ -13,10 +13,10 @@
#include <fcntl.h>
/* The full definition is in iomodule. We reproduce
enough here to get the handle, which is all we want. */
enough here to get the fd, which is all we want. */
typedef struct {
PyObject_HEAD
HANDLE handle;
int fd;
} winconsoleio;
@ -67,7 +67,10 @@ _testconsole_write_input_impl(PyObject *module, PyObject *file,
prec->Event.KeyEvent.uChar.UnicodeChar = *p;
}
HANDLE hInput = ((winconsoleio*)file)->handle;
HANDLE hInput = _Py_get_osfhandle(((winconsoleio*)file)->fd);
if (hInput == INVALID_HANDLE_VALUE)
goto error;
DWORD total = 0;
while (total < size) {
DWORD wrote;

View File

@ -177,19 +177,11 @@ static long
msvcrt_open_osfhandle_impl(PyObject *module, void *handle, int flags)
/*[clinic end generated code: output=b2fb97c4b515e4e6 input=d5db190a307cf4bb]*/
{
int fd;
if (PySys_Audit("msvcrt.open_osfhandle", "Ki", handle, flags) < 0) {
return -1;
}
_Py_BEGIN_SUPPRESS_IPH
fd = _open_osfhandle((intptr_t)handle, flags);
_Py_END_SUPPRESS_IPH
if (fd == -1)
PyErr_SetFromErrno(PyExc_OSError);
return fd;
return _Py_open_osfhandle(handle, flags);
}
/*[clinic input]
@ -207,19 +199,11 @@ static void *
msvcrt_get_osfhandle_impl(PyObject *module, int fd)
/*[clinic end generated code: output=aca01dfe24637374 input=5fcfde9b17136aa2]*/
{
intptr_t handle = -1;
if (PySys_Audit("msvcrt.get_osfhandle", "(i)", fd) < 0) {
return NULL;
}
_Py_BEGIN_SUPPRESS_IPH
handle = _get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
if (handle == -1)
PyErr_SetFromErrno(PyExc_OSError);
return (HANDLE)handle;
return _Py_get_osfhandle(fd);
}
/* Console I/O */

View File

@ -249,10 +249,8 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
if (!Py_LegacyWindowsStdioFlag && sys_stdin == stdin) {
HANDLE hStdIn, hStdErr;
_Py_BEGIN_SUPPRESS_IPH
hStdIn = (HANDLE)_get_osfhandle(fileno(sys_stdin));
hStdErr = (HANDLE)_get_osfhandle(fileno(stderr));
_Py_END_SUPPRESS_IPH
hStdIn = _Py_get_osfhandle_noraise(fileno(sys_stdin));
hStdErr = _Py_get_osfhandle_noraise(fileno(stderr));
if (_get_console_type(hStdIn) == 'r') {
fflush(sys_stdout);

View File

@ -1016,9 +1016,7 @@ _Py_fstat_noraise(int fd, struct _Py_stat_struct *status)
HANDLE h;
int type;
_Py_BEGIN_SUPPRESS_IPH
h = (HANDLE)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
h = _Py_get_osfhandle_noraise(fd);
if (h == INVALID_HANDLE_VALUE) {
/* errno is already set by _get_osfhandle, but we also set
@ -1157,9 +1155,7 @@ get_inheritable(int fd, int raise)
HANDLE handle;
DWORD flags;
_Py_BEGIN_SUPPRESS_IPH
handle = (HANDLE)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
handle = _Py_get_osfhandle_noraise(fd);
if (handle == INVALID_HANDLE_VALUE) {
if (raise)
PyErr_SetFromErrno(PyExc_OSError);
@ -1230,9 +1226,7 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works)
}
#ifdef MS_WINDOWS
_Py_BEGIN_SUPPRESS_IPH
handle = (HANDLE)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
handle = _Py_get_osfhandle_noraise(fd);
if (handle == INVALID_HANDLE_VALUE) {
if (raise)
PyErr_SetFromErrno(PyExc_OSError);
@ -2006,13 +2000,9 @@ _Py_dup(int fd)
assert(PyGILState_Check());
#ifdef MS_WINDOWS
_Py_BEGIN_SUPPRESS_IPH
handle = (HANDLE)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
if (handle == INVALID_HANDLE_VALUE) {
PyErr_SetFromErrno(PyExc_OSError);
handle = _Py_get_osfhandle(fd);
if (handle == INVALID_HANDLE_VALUE)
return -1;
}
Py_BEGIN_ALLOW_THREADS
_Py_BEGIN_SUPPRESS_IPH
@ -2122,8 +2112,47 @@ error:
PyErr_SetFromErrno(PyExc_OSError);
return -1;
}
#endif
#else /* MS_WINDOWS */
void*
_Py_get_osfhandle_noraise(int fd)
{
void *handle;
_Py_BEGIN_SUPPRESS_IPH
handle = (void*)_get_osfhandle(fd);
_Py_END_SUPPRESS_IPH
return handle;
}
void*
_Py_get_osfhandle(int fd)
{
void *handle = _Py_get_osfhandle_noraise(fd);
if (handle == INVALID_HANDLE_VALUE)
PyErr_SetFromErrno(PyExc_OSError);
return handle;
}
int
_Py_open_osfhandle_noraise(void *handle, int flags)
{
int fd;
_Py_BEGIN_SUPPRESS_IPH
fd = _open_osfhandle((intptr_t)handle, flags);
_Py_END_SUPPRESS_IPH
return fd;
}
int
_Py_open_osfhandle(void *handle, int flags)
{
int fd = _Py_open_osfhandle_noraise(handle, flags);
if (fd == -1)
PyErr_SetFromErrno(PyExc_OSError);
return fd;
}
#endif /* MS_WINDOWS */
int
_Py_GetLocaleconvNumeric(struct lconv *lc,