_PyIO_get_console_type fails for various paths

[eryksun]

BPO	28164
Nosy	@pfmoore, @tjguk, @zware, @eryksun, @zooba
PRs	[Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
Files	issue_28164_01.patch issue_28164_02.patch

^{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/zooba'
closed_at = <Date 2017-02-06.22:54:02.916>
created_at = <Date 2016-09-15.03:51:03.787>
labels = ['3.7', 'type-bug', 'library', 'expert-IO', 'OS-windows']
title = '_PyIO_get_console_type fails for various paths'
updated_at = <Date 2017-03-31.16:36:11.181>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:11.181>
actor = 'dstufft'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2017-02-06.22:54:02.916>
closer = 'steve.dower'
components = ['Library (Lib)', 'Windows', 'IO']
creation = <Date 2016-09-15.03:51:03.787>
creator = 'eryksun'
dependencies = []
files = ['46461', '46463']
hgrepos = []
issue_num = 28164
keywords = ['patch']
message_count = 17.0
messages = ['276512', '276839', '276844', '276864', '286507', '286516', '286522', '287005', '287014', '287019', '287027', '287030', '287047', '287177', '287178', '287179', '287180']
nosy_count = 6.0
nosy_names = ['paul.moore', 'tim.golden', 'python-dev', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28164'
versions = ['Python 3.6', 'Python 3.7']

[eryksun]

_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.

[zooba]

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.

[python-dev]

New changeset d0bab9fda568 by Steve Dower in branch '3.6':
Issue bpo-28161: Opening CON for write access fails
https://hg.python.org/cpython/rev/d0bab9fda568

New changeset 187a114b9ef4 by Steve Dower in branch 'default':
Issue bpo-28161: Opening CON for write access fails
https://hg.python.org/cpython/rev/187a114b9ef4

[eryksun]

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.

[eryksun]

I had reopened this issue with a suggestion for expanding the supported paths in msg276864, 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.

[zooba]

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.

[eryksun]

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.