This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Created on 2016-09-15 03:25 by eryksun, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue_28162_01.patch eryksun, 2016-09-21 05:46 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (7)
msg276508 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2016-09-15 03:25
For a console readall(), if the first line starts with '\x1a' (i.e. Ctrl+Z), it breaks out of its read loop before incrementing len. Thus the input isn't handled properly as EOF, for which the check requires len > 0. Instead it ends up calling WideCharToMultiByte with len == 0, which fails as follows: >>> sys.stdin.buffer.raw.read() ^Z Traceback (most recent call last): File "<stdin>", line 1, in <module> OSError: [WinError 87] The parameter is incorrect
msg276841 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-09-17 20:38
Unfortunately, very hard to test this because it requires interacting with an actual console. But it's an easy fix.
msg276845 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-17 20:51
New changeset d0bab9fda568 by Steve Dower in branch '3.6': Issue #28161: Opening CON for write access fails https://hg.python.org/cpython/rev/d0bab9fda568 New changeset 187a114b9ef4 by Steve Dower in branch 'default': Issue #28161: Opening CON for write access fails https://hg.python.org/cpython/rev/187a114b9ef4
msg276848 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-09-17 20:54
The fix here was changing "n > 0 && buf[len] == '\x1a'" into "n == 0 || buf[len] == '\x1a'" when returning an empty bytes. Previously we were falling into the conversion code which wasn't happy with n == 0.
msg277093 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2016-09-21 05:46
For breaking out of the readall while loop, you only need to check if the current read is empty: /* when the read is empty we break */ if (n == 0) break; Also, the logic is wrong here: if (len == 0 || buf[0] == '\x1a' && _buflen(self) == 0) { /* when the result starts with ^Z we return an empty buffer */ PyMem_Free(buf); return PyBytes_FromStringAndSize(NULL, 0); } This is true when len is 0 or when buf[0] is Ctrl+Z and _buflen(self) is 0. Since buf[0] shouldn't ever be Ctrl+Z here (low-level EOF handling is abstracted in read_console_w), it's never checking the internal buffer. We can easily see this going wrong here: >>> a = sys.stdin.buffer.raw.read(1); b = sys.stdin.buffer.raw.read() Ā^Z >>> a b'\xc4' >>> b b'' It misses the remaining byte in the internal buffer. This check can be simplified as follows: rn = _buflen(self); if (len == 0 && rn == 0) { /* return an empty buffer */ PyMem_Free(buf); return PyBytes_FromStringAndSize(NULL, 0); } After this the code assumes that len isn't 0, which leads to more WideCharToMultiByte failure cases. In the last conversion it's overwrite bytes_size without including rn. I'm not sure what's going on with _PyBytes_Resize(&bytes, n * sizeof(wchar_t)). ISTM, it should be resized to bytes_size, and make sure this includes rn. Finally, _copyfrombuf is repeatedly overwriting buf[0] instead of writing to buf[n]. With the attached patch, the behavior seems correct now: >>> sys.stdin.buffer.raw.read() ^Z b'' >>> sys.stdin.buffer.raw.read() abc^Z ^Z b'abc\x1a\r\n' Split U+0100: >>> a = sys.stdin.buffer.raw.read(1); b = sys.stdin.buffer.raw.read() Ā^Z >>> a b'\xc4' >>> b b'\x80' Split U+1234: >>> a = sys.stdin.buffer.raw.read(1); b = sys.stdin.buffer.raw.read() ሴ^Z >>> a b'\xe1' >>> b b'\x88\xb4' The buffer still can't handle splitting an initial non-BMP character, stored as a surrogate pair. Both codes end up as replacement characters because they aren't transcoded as a unit. Split U+00010000: >>> a = sys.stdin.buffer.raw.read(1); b = sys.stdin.buffer.raw.read() 𐀀^Z ^Z >>> a b'\xef' >>> b b'\xbf\xbd\xef\xbf\xbd\x1a\r\n'
msg278320 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-08 19:38
New changeset 4d4aefa52f49 by Steve Dower in branch '3.6': Issue #28162: Fixes Ctrl+Z handling in console readall() https://hg.python.org/cpython/rev/4d4aefa52f49 New changeset 947fa496ca6f by Steve Dower in branch 'default': Issue #28162: Fixes Ctrl+Z handling in console readall() https://hg.python.org/cpython/rev/947fa496ca6f
msg278321 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-10-08 19:44
I didn't see your patch, but I made basically the same fix and added a test.
History
Date User Action Args
2022-04-11 14:58:36adminsetgithub: 72349
2017-03-31 16:36:32dstufftsetpull_requests: + pull_request1057
2016-10-08 19:44:31steve.dowersetstatus: open -> closed

messages: + msg278321
2016-10-08 19:38:08python-devsetmessages: + msg278320
2016-09-21 05:46:33eryksunsetstatus: closed -> open
files: + issue_28162_01.patch
messages: + msg277093

keywords: + patch
2016-09-17 20:54:15steve.dowersetstatus: open -> closed
resolution: fixed
messages: + msg276848

stage: needs patch -> resolved
2016-09-17 20:51:44python-devsetnosy: + python-dev
messages: + msg276845
2016-09-17 20:38:02steve.dowersetassignee: steve.dower
messages: + msg276841
stage: test needed -> needs patch
2016-09-15 03:25:17eryksuncreate