From 2e8166c7d1c831a87a2b046295cdc753dcd31c4e Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 22 Aug 2023 00:39:47 +0100 Subject: [PATCH 1/2] gh-106242: Make ntpath.realpath errors consistent with abspath when there are embedded nulls --- Lib/ntpath.py | 12 ++++++++++++ Lib/test/test_ntpath.py | 6 ++++++ .../2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst | 3 +++ 3 files changed, 21 insertions(+) create mode 100644 Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst diff --git a/Lib/ntpath.py b/Lib/ntpath.py index dadcdc0c495da1..df3402d46c9cc6 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -721,6 +721,14 @@ def realpath(path, *, strict=False): try: path = _getfinalpathname(path) initial_winerror = 0 + except ValueError as ex: + # gh-106242: Raised for embedded null characters + # In strict mode, we convert into an OSError. + # Non-strict mode returns the path as-is, since we've already + # made it absolute. + if strict: + raise OSError(str(ex)) from None + path = normpath(path) except OSError as ex: if strict: raise @@ -740,6 +748,10 @@ def realpath(path, *, strict=False): try: if _getfinalpathname(spath) == path: path = spath + except ValueError as ex: + # Unexpected, as an invalid path should not have gained a prefix + # at any point, but we ignore this error just in case. + pass except OSError as ex: # If the path does not exist and originally did not exist, then # strip the prefix anyway. diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 78e1cb582512b0..d91dcdfb0c5fac 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -394,6 +394,10 @@ def test_realpath_basic(self): d = drives.pop().encode() self.assertEqual(ntpath.realpath(d), d) + # gh-106242: Embedded nulls and non-strict fallback to abspath + self.assertEqual(ABSTFN + "\0spam", + ntpath.realpath(os_helper.TESTFN + "\0spam", strict=False)) + @os_helper.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') def test_realpath_strict(self): @@ -404,6 +408,8 @@ def test_realpath_strict(self): self.addCleanup(os_helper.unlink, ABSTFN) self.assertRaises(FileNotFoundError, ntpath.realpath, ABSTFN, strict=True) self.assertRaises(FileNotFoundError, ntpath.realpath, ABSTFN + "2", strict=True) + # gh-106242: Embedded nulls should raise OSError (not ValueError) + self.assertRaises(OSError, ntpath.realpath, ABSTFN + "\0spam", strict=True) @os_helper.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') diff --git a/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst b/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst new file mode 100644 index 00000000000000..31958de2bfbf1a --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst @@ -0,0 +1,3 @@ +Fixes :func:`~os.path.realpath` to behave consistently when passed a path +containing an embedded null character. In strict mode, it now raises +:exc:`OSError`, and in non-strict mode will make the path absolute. From d526376dde6701771e692f0b25ae1ba2af50a90a Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 21 Aug 2023 16:46:14 -0700 Subject: [PATCH 2/2] Update 2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst mention Windows and the former incorrect ValueError. --- .../Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst b/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst index 31958de2bfbf1a..ffe42ec5dc3faf 100644 --- a/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst +++ b/Misc/NEWS.d/next/Windows/2023-08-22-00-36-57.gh-issue-106242.q24ITw.rst @@ -1,3 +1,4 @@ Fixes :func:`~os.path.realpath` to behave consistently when passed a path -containing an embedded null character. In strict mode, it now raises -:exc:`OSError`, and in non-strict mode will make the path absolute. +containing an embedded null character on Windows. In strict mode, it now +raises :exc:`OSError` instead of the unexpected :exc:`ValueError`, and in +non-strict mode will make the path absolute.