| msg276512 - (view) |
Author: Eryk Sun (eryksun) *  |
Date: 2016-09-15 03:51 |
| _PyIO_get_console_type currently hard codes the names "CON", "CONIN$", and "CONOUT$" and doesn't use a case-insensitive comparison. For example, opening "conin$" doesn't get directed to WindowsConsoleIO: >>> open('conin$', 'rb', buffering=0) <_io.FileIO name='conin$' mode='rb' closefd=True> unlike CONIN$: >>> open('CONIN$', 'rb', buffering=0) <_io._WindowsConsoleIO mode='rb' closefd=True> This also ignores the special handling of DOS devices in existing directories. The normal DOS-device check (i.e. if the parent directory exists, call GetFullPathName to normalize the path) isn't reliable, unfortunately. Prior to Windows 8, CreateFile special-cases parsing console paths by calling BaseIsThisAConsoleName, which has an annoying speed hack that makes it buggy. Ultimately the only way to know if a path opens the console is to open it and check the handle. |
|
|
| msg276839 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2016-09-17 20:29 |
| I'll make it case insensitive, but unfortunately I can't find a good way to check that a handle is actually a real console handle or what type it is. Plus the way iomodule is designed we'd need to open it twice, which I'd rather not do. |
|
|
| msg276844 - (view) |
Author: Roundup Robot (python-dev) |
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 |
|
|
| msg276864 - (view) |
Author: Eryk Sun (eryksun) * |
Date: 2016-09-18 01:08 |
| > check that a handle is actually a real console handle or > what type it is Did you mean a path here? Certainly you can check a handle, but that means opening the path twice. You can use GetFullPathName to classify the path, and then use GetFullPathName again when opening it. Keep the original path as the `name`. When classifying the path, ignore the DOS-device prefix, "\\.\" or "\\?\", if present, and do a case-insensitive match for CON, CONIN$, or CONOUT$. GetFullPathName transforms paths containing the "CON" device, e.g. "C:\Temp\con :.spam " => "\\.\CON". This ignores trailing spaces and anything after the first "." or ":" in the last component. It also translates forward slashes, e.g. "//./con" => "\\.\con". On Windows 8+ it also works with CONIN$ and CONOUT$, transforming them to "\\.\CONIN$" and "\\.\CONOUT$". This is reliable on Windows 8+, without having to also call GetFullPathName again when opening the path. The problem with Windows Vista and 7 is a speed hack it has. In these older versions, opening the console has to be handled specially by the function OpenConsoleW, so like you have to do here, in Windows 7 there's an internal BaseIsThisAConsoleName function to classify a path. This function always checks for "CON", "CONIN$", or "CONOUT$" (case insensitive). But it also matches "\\.\CON" and in some cases also "CON" in the last component. Since BaseIsThisAConsoleName always has to be called, the path search could hurt performance. So there's a dubious hack to only look for "CON" if the path starts with "\", "c", or "C" or ends with "n", "N", "or ":". Thus "C:\temp\con " opens the console because it starts with "C", but not "Z:\temp\con " because it starts with "Z" and ends with a space. |
|
|
| msg286507 - (view) |
Author: Eryk Sun (eryksun) * |
Date: 2017-01-31 03:20 |
| I had reopened this issue with a suggestion for expanding the supported paths in , but it's languished for a while now. I've attached a patch implementing the change to _PyIO_get_console_type that I had suggested. Here are some example paths that this change enables (tested in Windows 10; need to check Windows 7): \\.\conin$ \\.\conout$ \\.\con //?/con C:/Temp/con C:/Temp/conout$ C:/Temp/conin$ Paths such as "C:/Temp/con" are recognized by first converting to a device path such as r"\\.\con". It's fine if the directory doesn't exist because CreateFile will fail anyway in that case. This patch also fixes an error in handling the return value of _get_osfhandle when it fails. It also adds a couple checks to __init__ to provide a better error message when io._WindowsConsoleIO is called directly for some reason instead of via open(). If the console type can't be determined it should error out immediately instead of printing an unrelated error about not being able to open the input/output buffer. |
|
|
| msg286516 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2017-01-31 04:57 |
| I'm okay with this patch. We move closer to being right without degrading the normal case, and I don't think the edge cases are important (and the behavior in those cases will be acceptable). With a couple of tests to make sure the path comparisons don't get broken in the future, this will be ready to merge. |
|
|
| msg286522 - (view) |
Author: Eryk Sun (eryksun) * |
Date: 2017-01-31 12:23 |
| I added some tests to ensure open() returns an instance of _WindowsConsoleIO for a few console paths. It also checks that opening a non-console file raises a ValueError, both for a path and an fd, as well for a negative file descriptor. |
|
|
| msg287005 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2017-02-04 23:14 |
| New changeset ed0c05c739c9 by Steve Dower in branch '3.6': Issue #28164: Correctly handle special console filenames (patch by Eryk Sun) https://hg.python.org/cpython/rev/ed0c05c739c9 New changeset a5538734cc87 by Steve Dower in branch 'default': Merge issue #28164 and issue #29409 https://hg.python.org/cpython/rev/a5538734cc87 |
|
|
| msg287014 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2017-02-05 00:00 |
| New changeset 164fc49825081694c9c44c4a17c75b30c5ba8f51 by Steve Dower in branch 'master': Issue #28164: Correctly handle special console filenames (patch by Eryk Sun) https://github.com/python/cpython/commit/164fc49825081694c9c44c4a17c75b30c5ba8f51 New changeset 965f8d68a8dcc2ebb2480fe7e9121ac7efdee54e by Steve Dower in branch 'master': Merge issue #28164 and issue #29409 https://github.com/python/cpython/commit/965f8d68a8dcc2ebb2480fe7e9121ac7efdee54e |
|
|
| msg287019 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2017-02-05 00:00 |
| New changeset 164fc49825081694c9c44c4a17c75b30c5ba8f51 by Steve Dower in branch '3.6': Issue #28164: Correctly handle special console filenames (patch by Eryk Sun) https://github.com/python/cpython/commit/164fc49825081694c9c44c4a17c75b30c5ba8f51 |
|
|
| msg287027 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2017-02-05 00:30 |
| Looks like the "need to check Win7" part was actually important... there are buildbot failures from this. I *think* they only require test handling updates. |
|
|
| msg287030 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2017-02-05 01:30 |
| Okay, seems like Windows 7 GetFullPathName on conin$ and conout$ returns the path appended to the current directory, and you need to specify the full name as "conin$" or "conout$" to access the console - otherwise you have a file by that name. We should include a comparison for these names exactly before using GetFullPathName. |
|
|
| msg287047 - (view) |
Author: Eryk Sun (eryksun) * |
Date: 2017-02-05 12:56 |
| It's an ugly inconsistency that GetFullPathName fails for bare CONIN$ and CONOUT$ prior to Windows 8, in that it gives a different result from simply passing those names to CreateFile. Anyway, thanks for modifying it to work correctly in this case. We should test that _WindowsConsoleIO isn't used on Windows 7 and earlier when accessing CONIN$ or CONOUT$ in a directory. How about the following? temp_path = tempfile.mkdtemp() self.addCleanup(support.rmtree, temp_path) conout_path = os.path.join(temp_path, 'CONOUT$') with open(conout_path, 'wb', buffering=0) as f: if sys.getwindowsversion()[:2] > (6, 1): self.assertIsInstance(f, ConIO) else: self.assertNotIsInstance(f, ConIO) |
|
|
| msg287177 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2017-02-06 22:53 |
| New changeset 4463e311f5bd by Steve Dower in branch '3.6': Issue #28164: Improves test on Windows 7 https://hg.python.org/cpython/rev/4463e311f5bd New changeset 8132bcc1522d by Steve Dower in branch 'default': Issue #28164: Improves test on Windows 7 https://hg.python.org/cpython/rev/8132bcc1522d |
|
|
| msg287178 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2017-02-06 22:54 |
| That test looks good for me, and I verified it on both Win7 and Win10. (Hopefully we don't have any Win8.1 edge cases in here...) |
|
|
| msg287179 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2017-02-06 23:00 |
| New changeset c6c32889c9d80ffd599a57fd0d4c4a88deece29b by Steve Dower in branch 'master': Issue #28164: Improves test on Windows 7 https://github.com/python/cpython/commit/c6c32889c9d80ffd599a57fd0d4c4a88deece29b New changeset dc5c4bc90770d6a5875666434cf797c77eb3dcad by Steve Dower in branch 'master': Issue #28164: Improves test on Windows 7 https://github.com/python/cpython/commit/dc5c4bc90770d6a5875666434cf797c77eb3dcad |
|
|
| msg287180 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2017-02-06 23:00 |
| New changeset c6c32889c9d80ffd599a57fd0d4c4a88deece29b by Steve Dower in branch '3.6': Issue #28164: Improves test on Windows 7 https://github.com/python/cpython/commit/c6c32889c9d80ffd599a57fd0d4c4a88deece29b |
|
|