os.readlink fails on Windows

[eryksun]

BPO	29248
Nosy	@pfmoore, @tjguk, @berkerpeksag, @zware, @eryksun, @zooba, @miss-islington, @SSE4
PRs	bpo-29248: Fix readlink bug os #5577 [3.6] bpo-29248: Fix os.readlink() on Windows (GH-5577) #5640 [3.7] bpo-29248: Fix os.readlink() on Windows (GH-5577) #5644
Files	issue29248.patch issue29248-2.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 = None
closed_at = <Date 2018-02-12.22:47:00.913>
created_at = <Date 2017-01-12.08:09:20.879>
labels = ['3.7', '3.8', 'type-bug', 'library', 'OS-windows']
title = 'os.readlink fails on Windows'
updated_at = <Date 2018-02-12.22:47:00.911>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2018-02-12.22:47:00.911>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2018-02-12.22:47:00.913>
closer = 'berker.peksag'
components = ['Library (Lib)', 'Windows']
creation = <Date 2017-01-12.08:09:20.879>
creator = 'eryksun'
dependencies = []
files = ['46290', '46293']
hgrepos = []
issue_num = 29248
keywords = ['patch']
message_count = 9.0
messages = ['285294', '285501', '286466', '286469', '311781', '312063', '312067', '312082', '312088']
nosy_count = 9.0
nosy_names = ['paul.moore', 'tim.golden', 'craigh', 'berker.peksag', 'zach.ware', 'eryksun', 'steve.dower', 'miss-islington', 'SSE4']
pr_nums = ['5577', '5640', '5644']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29248'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[eryksun]

win_readlink in Modules/posixmodule.c mistakenly treats the PrintNameOffset field of the reparse data buffer as a number of characters instead of bytes. Thus, if the offset is non-zero, the value returned is incorrect stack garbage. For example, the following should return "C:\\ProgramData":

    >>> os.readlink(r'C:\Users\All Users')
    '\u6c20\u3012\u041f\x01\u2768\u60b2\u031b\x02\x05\x00\u031e\x06\u8c01\u4012'

Craig Holmquist found this bug, as detailed in message 277385. He included a fix in the patch for bpo-23407, but this should be addressed in the next maintenance release of 3.5 and 3.6.

[craigh]

New patch with test.

I'm not sure if C:\Users\All Users and C:\ProgramData exist with those names on non-English installations of Windows. I set the test to skip if they aren't found.

[berkerpeksag]

Thanks for the patch. I think it would be better to use the existing framework in Win32SymlinkTests to create a test file instead of using a hardcoded path like 'C:\Users\All Users' (e.g. use Win32SymlinkTests.filelink and Win32SymlinkTests.filelink_target to create a link)

[eryksun]

os.symlink calls CreateSymbolicLink, which creates the reparse data buffer with the print name stored first, so the offset is always 0. Otherwise we would have noticed this problem already. For example:

    >>> os.symlink('C:\\', 'link', True)
    >>> os.system('fsutil reparsepoint query link')
    Reparse Tag Value : 0xa000000c
    Tag value: Microsoft
    Tag value: Name Surrogate
    Tag value: Symbolic Link

Reparse Data Length: 0x00000020
Reparse Data:
0000:  06 00 0e 00 00 00 06 00  00 00 00 00 43 00 3a 00  ............C.:.
0010:  5c 00 5c 00 3f 00 3f 00  5c 00 43 00 3a 00 5c 00  \.\.?.?.\.C.:.\.

As you can see above, CreateSymbolicLink stores the "PrintName" DOS path (e.g. "C:\") first, with an offset of 0, followed by the "SubstituteName" NT path (e.g. "\??\C:\").

The "All Users" link, on the other hand, seems to have been created manually with an inverted order. I have ctypes code to manually create a similar link (calling OpenProcessToken/AdjustTokenPrivileges to enable the required privilege and CreateFile/DeviceIoControl to set the reparse point), but I doubt it's worth adding it just to test this bug.

[SSE4]

opened PR #5577

[berkerpeksag]

New changeset 3c34aad by Berker Peksag (SSE4) in branch 'master':
bpo-29248: Fix os.readlink() on Windows (GH-5577)
3c34aad

[berkerpeksag]

New changeset a1d33f7 by Berker Peksag (Miss Islington (bot)) in branch '3.6':
bpo-29248: Fix os.readlink() on Windows (GH-5577)
a1d33f7

[miss-islington]

New changeset 74ebbae by Miss Islington (bot) in branch '3.7':
bpo-29248: Fix os.readlink() on Windows (GH-5577)
74ebbae

[berkerpeksag]

Thank you, Craig and SSE4.