From 2cb8ba656ce67861289fc16d218ef10fb076c30b Mon Sep 17 00:00:00 2001 From: SSE4 Date: Wed, 7 Feb 2018 17:01:17 +0700 Subject: [PATCH 1/8] bpo-29248: Fix readlink bug os Signed-off-by: SSE4 --- Lib/test/test_os.py | 8 ++++++++ Modules/posixmodule.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 02611d223b7665..d7efc006af1e9e 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2165,6 +2165,14 @@ def test_12084(self): finally: os.chdir(orig_dir) + @unittest.skipUnless(os.path.lexists(r'C:\Users\All Users') + and os.path.exists(r'C:\ProgramData'), + 'Test directories not found') + def test_29248(self): + target = os.readlink(r'C:\Users\All Users') + + self.assertTrue(os.path.samefile(target, r'C:\ProgramData')) + @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests") class Win32JunctionTests(unittest.TestCase): diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 84b5b993cdfc13..0bf2e2932f5fa1 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7440,7 +7440,7 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs) return NULL; } print_name = rdb->SymbolicLinkReparseBuffer.PathBuffer + - rdb->SymbolicLinkReparseBuffer.PrintNameOffset; + (rdb->SymbolicLinkReparseBuffer.PrintNameOffset / 2); result = PyUnicode_FromWideChar(print_name, rdb->SymbolicLinkReparseBuffer.PrintNameLength/2); From 3c8ce39ce59a11d52fe4394de12962d50cd320a2 Mon Sep 17 00:00:00 2001 From: SSE4 Date: Wed, 7 Feb 2018 17:51:42 +0700 Subject: [PATCH 2/8] add Misc/NEWS.d entry Signed-off-by: SSE4 --- .../NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst diff --git a/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst b/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst new file mode 100644 index 00000000000000..75fc8b0e119ad6 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst @@ -0,0 +1 @@ +fix os.readlink() on Windows From 9893c55ba44cf16243b221bfea8eb48bc92f58a3 Mon Sep 17 00:00:00 2001 From: SSE4 Date: Wed, 7 Feb 2018 23:47:38 +0700 Subject: [PATCH 3/8] add explanation from the tracker issue --- Lib/test/test_os.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index d7efc006af1e9e..af518ec7cc4055 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2169,6 +2169,10 @@ def test_12084(self): and os.path.exists(r'C:\ProgramData'), 'Test directories not found') def test_29248(self): + # os.symlink calls CreateSymbolicLink, which creates the reparse data buffer with the print name stored first, + # so the offset is always 0. 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. target = os.readlink(r'C:\Users\All Users') self.assertTrue(os.path.samefile(target, r'C:\ProgramData')) From d23bd992322b4bdede832542aa6d01a78f7c219a Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Wed, 7 Feb 2018 20:17:32 +0300 Subject: [PATCH 4/8] Cosmetic fixes --- Lib/test/test_os.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index af518ec7cc4055..77e4a008ae61e8 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2169,12 +2169,15 @@ def test_12084(self): and os.path.exists(r'C:\ProgramData'), 'Test directories not found') def test_29248(self): - # os.symlink calls CreateSymbolicLink, which creates the reparse data buffer with the print name stored first, - # so the offset is always 0. 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. + # os.symlink() calls CreateSymbolicLink, which creates + # the reparse data buffer with the print name stored + # first, so the offset is always 0. 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. target = os.readlink(r'C:\Users\All Users') - self.assertTrue(os.path.samefile(target, r'C:\ProgramData')) From b0e82eb9786da8ca5db0b545dafbd70033e53799 Mon Sep 17 00:00:00 2001 From: SSE4 Date: Thu, 8 Feb 2018 00:26:44 +0700 Subject: [PATCH 5/8] make the news entry more descriptive --- .../next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst b/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst index 75fc8b0e119ad6..0e81c125598200 100644 --- a/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst +++ b/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst @@ -1 +1,3 @@ -fix os.readlink() on Windows +fix os.readlink() on Windows, which was mistakenly treating the +PrintNameOffset field of the reparse data buffer as a number of +characters instead of bytes. Patch by Craig Holmquist and SSE4. From a816bcc34f7096970dd47868e46414aacac5c993 Mon Sep 17 00:00:00 2001 From: SSE4 Date: Thu, 8 Feb 2018 09:48:21 +0700 Subject: [PATCH 6/8] use sizeof(wchar_t) instead of hard coding 2 --- Modules/posixmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0bf2e2932f5fa1..1412bdc21244df 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7443,7 +7443,7 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs) (rdb->SymbolicLinkReparseBuffer.PrintNameOffset / 2); result = PyUnicode_FromWideChar(print_name, - rdb->SymbolicLinkReparseBuffer.PrintNameLength/2); + rdb->SymbolicLinkReparseBuffer.PrintNameLength / sizeof(wchar_t)); return result; } From 3234a71d2bc0e713a9058f100c5ce8c9fc9192c0 Mon Sep 17 00:00:00 2001 From: SSE4 Date: Thu, 8 Feb 2018 09:49:15 +0700 Subject: [PATCH 7/8] PrintNameOffset can be odd, use two casts to have the compiler do the work --- Modules/posixmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 1412bdc21244df..72fda6a2196670 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7439,8 +7439,8 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs) "not a symbolic link"); return NULL; } - print_name = rdb->SymbolicLinkReparseBuffer.PathBuffer + - (rdb->SymbolicLinkReparseBuffer.PrintNameOffset / 2); + print_name = (wchar_t *)((char*)rdb->SymbolicLinkReparseBuffer.PathBuffer + + rdb->SymbolicLinkReparseBuffer.PrintNameOffset); result = PyUnicode_FromWideChar(print_name, rdb->SymbolicLinkReparseBuffer.PrintNameLength / sizeof(wchar_t)); From 0d66fb311fa10ce55d56c7e01b62b977814cf548 Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Fri, 9 Feb 2018 10:40:08 +0300 Subject: [PATCH 8/8] use reST format in NEWS entry --- .../next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst b/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst index 0e81c125598200..3030ef6958de0f 100644 --- a/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst +++ b/Misc/NEWS.d/next/Windows/2018-02-07-17-50-48.bpo-29248.Xzwj-6.rst @@ -1,3 +1,3 @@ -fix os.readlink() on Windows, which was mistakenly treating the -PrintNameOffset field of the reparse data buffer as a number of +Fix :func:`os.readlink` on Windows, which was mistakenly treating the +``PrintNameOffset`` field of the reparse data buffer as a number of characters instead of bytes. Patch by Craig Holmquist and SSE4.