classification
Title: os.readlink fails on Windows
Type: behavior Stage: resolved
Components: Library (Lib), Windows Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: SSE4, berker.peksag, craigh, eryksun, miss-islington, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2017-01-12 08:09 by eryksun, last changed 2018-02-12 22:47 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
issue29248.patch craigh, 2017-01-14 15:31 review
issue29248-2.patch craigh, 2017-01-14 20:38 review
Pull Requests
URL Status Linked Edit
PR 5577 merged python-dev, 2018-02-07 10:07
PR 5640 merged miss-islington, 2018-02-12 17:12
PR 5644 merged miss-islington, 2018-02-12 21:02
Messages (9)
msg285294 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-01-12 08:09
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 issue 23407, but this should be addressed in the next maintenance release of 3.5 and 3.6.
msg285501 - (view) Author: Craig Holmquist (craigh) Date: 2017-01-14 20:38
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.
msg286466 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-01-30 02:20
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)
msg286469 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-01-30 03:29
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.
msg311781 - (view) Author: SSE4 (SSE4) * Date: 2018-02-07 10:16
opened PR https://github.com/python/cpython/pull/5577
msg312063 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-02-12 17:10
New changeset 3c34aad4e7a95913ec7db8e5e948a8fc69047bf7 by Berker Peksag (SSE4) in branch 'master':
bpo-29248: Fix os.readlink() on Windows (GH-5577)
https://github.com/python/cpython/commit/3c34aad4e7a95913ec7db8e5e948a8fc69047bf7
msg312067 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-02-12 18:14
New changeset a1d33f742515dc70ae99bc3ea1c851729522afc3 by Berker Peksag (Miss Islington (bot)) in branch '3.6':
bpo-29248: Fix os.readlink() on Windows (GH-5577)
https://github.com/python/cpython/commit/a1d33f742515dc70ae99bc3ea1c851729522afc3
msg312082 - (view) Author: miss-islington (miss-islington) Date: 2018-02-12 21:39
New changeset 74ebbaeb566dc10031756430ec5c896e56d0e491 by Miss Islington (bot) in branch '3.7':
bpo-29248: Fix os.readlink() on Windows (GH-5577)
https://github.com/python/cpython/commit/74ebbaeb566dc10031756430ec5c896e56d0e491
msg312088 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-02-12 22:47
Thank you, Craig and SSE4.
History
Date User Action Args
2018-02-12 22:47:00berker.peksagsetstatus: open -> closed
versions: + Python 3.8
messages: + msg312088

resolution: fixed
stage: patch review -> resolved
2018-02-12 21:39:47miss-islingtonsetnosy: + miss-islington
messages: + msg312082
2018-02-12 21:02:09miss-islingtonsetpull_requests: + pull_request5445
2018-02-12 18:14:10berker.peksagsetmessages: + msg312067
2018-02-12 17:14:15berker.peksagsetversions: - Python 3.5
2018-02-12 17:12:19miss-islingtonsetpull_requests: + pull_request5440
2018-02-12 17:10:43berker.peksagsetmessages: + msg312063
2018-02-07 10:16:16SSE4setnosy: + SSE4
messages: + msg311781
2018-02-07 10:07:24python-devsetpull_requests: + pull_request5395
2017-01-30 03:29:11eryksunsetmessages: + msg286469
2017-01-30 02:20:31berker.peksagsetnosy: + berker.peksag

messages: + msg286466
stage: needs patch -> patch review
2017-01-14 20:38:49craighsetfiles: + issue29248-2.patch
nosy: + craigh
messages: + msg285501

2017-01-14 20:11:02eryksunlinkissue23407 dependencies
2017-01-14 15:31:45craighsetfiles: + issue29248.patch
keywords: + patch
2017-01-12 08:09:20eryksuncreate