[issue28164] _PyIO_get_console_type fails for various paths

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +865

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:


New changeset c6c32889c9d80ffd599a57fd0d4c4a88deece29b by Steve Dower in branch 
'3.6':
Issue #28164: Improves test on Windows 7
https://github.com/python/cpython/commit/c6c32889c9d80ffd599a57fd0d4c4a88deece29b


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Steve Dower]

Steve Dower added the comment:

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

--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

Eryk Sun added the comment:

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)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Steve Dower]

Steve Dower added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Steve Dower]

Steve Dower added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

Changes by Eryk Sun :


Added file: http://bugs.python.org/file46463/issue_28164_02.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

Changes by Eryk Sun :


Removed file: http://bugs.python.org/file46462/issue_28164_02.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

Eryk Sun added the comment:

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.

--
Added file: http://bugs.python.org/file46462/issue_28164_02.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Steve Dower]

Steve Dower added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

Eryk Sun added the comment:

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.

--
keywords: +patch
Added file: http://bugs.python.org/file46461/issue_28164_01.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

Eryk Sun added the comment:

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

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Steve Dower]

Changes by Steve Dower :


--
resolution:  -> fixed
stage: test needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Steve Dower]

Steve Dower added the comment:

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.

--
assignee:  -> steve.dower

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28164] _PyIO_get_console_type fails for various paths

[Eryk Sun]

New submission from Eryk Sun:

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

--
components: IO, Library (Lib), Windows
messages: 276512
nosy: eryksun, paul.moore, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
stage: test needed
status: open
title: _PyIO_get_console_type fails for various paths
type: behavior
versions: Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com