Jan 16, 2024 · Jan 2, 2024 · Jan 2, 2024 · Jan 2, 2024 · Jan 2, 2024 · Jan 5, 2024
diff --git a/Lib/site.py b/Lib/site.py
 import builtins
 import _sitebuiltins
 import io
 import stat

 # Prefixes for site-packages; add additional prefixes like /usr/local here
 PREFIXES = [sys.prefix, sys.exec_prefix]
    else:
        reset = False
    fullname = os.path.join(sitedir, name)
    try:
        st = os.lstat(fullname)
    except OSError:
        return
    if ((getattr(st, 'st_flags', 0) & stat.UF_HIDDEN) or
        (getattr(st, 'st_file_attributes', 0) & stat.FILE_ATTRIBUTE_HIDDEN)):
        _trace(f"Skipping hidden .pth file: {fullname!r}")
        return
    _trace(f"Processing .pth file: {fullname!r}")
    try:
        # locale encoding is not ideal especially on Windows. But we have used
        names = os.listdir(sitedir)
    except OSError:
        return
    names = [name for name in names if name.endswith(".pth")]
    names = [name for name in names
             if name.endswith(".pth") and not name.startswith(".")]
    for name in sorted(names):
        addpackage(sitedir, name, known_paths)
    if reset:
diff --git a/Lib/test/test_site.py b/Lib/test/test_site.py
        finally:
            pth_file.cleanup()

    def test_addsitedir_dotfile(self):
        pth_file = PthFile('.dotfile')
        pth_file.cleanup(prep=True)
        try:
            pth_file.create()
            site.addsitedir(pth_file.base_dir, set())
            self.assertNotIn(site.makepath(pth_file.good_dir_path)[0], sys.path)
            self.assertIn(pth_file.base_dir, sys.path)
        finally:
            pth_file.cleanup()

    @unittest.skipUnless(hasattr(os, 'chflags'), 'test needs os.chflags()')
    def test_addsitedir_hidden_pth_file(self):
        pth_file = PthFile()
        pth_file.cleanup(prep=True)
        try:
            pth_file.create()
            st = os.stat(pth_file.file_path)
            os.chflags(target_file, st.st_flags | stat.UF_IMMUTABLE)
            site.addsitedir(pth_file.base_dir, set())
            self.assertNotIn(site.makepath(pth_file.good_dir_path)[0], sys.path)
            self.assertIn(pth_file.base_dir, sys.path)
        finally:
            pth_file.cleanup()

    # This tests _getuserbase, hence the double underline
    # to distinguish from a test for getuserbase
    def test__getuserbase(self):
diff --git a/Misc/NEWS.d/next/Security/2024-01-02-19-52-23.gh-issue-113659.DkmnQc.rst b/Misc/NEWS.d/next/Security/2024-01-02-19-52-23.gh-issue-113659.DkmnQc.rst
 Skip ``.pth`` files with names starting with a dot or hidden file attribute.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -74,6 +74,7 @@
		import builtins
		import _sitebuiltins
		import io
		import stat

		# Prefixes for site-packages; add additional prefixes like /usr/local here
		PREFIXES = [sys.prefix, sys.exec_prefix]
Expand DownExpand Up		@@ -168,6 +169,14 @@ def addpackage(sitedir, name, known_paths):
		else:
		reset = False
		fullname = os.path.join(sitedir, name)
		try:
		st = os.lstat(fullname)
		except OSError:
Copy link Member vstinnerJan 10, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Hum, maybe add a _trace() call to help debugging such issue. Something like: `_trace(f"Skipping .pth file, stat failed: {exc!r}")` Copy link MemberAuthor serhiy-storchakaJan 11, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. When`open()` fails below, it is simply silently returns. If we want to add a`_trace()` for OSError, we should do this in both cases (and maybe in more cases). But this is a different issue. It can add a noise where it was silent. Copy link Member vstinnerJan 16, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Ah, I didn't notice. In this case, would you mind to add a comment (in the 2 places) to explain that ignoring silently the file on OSError is a deliberate choice? Copy link MemberAuthor serhiy-storchakaJan 16, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Most`OSError`s are silently ignored in this module and do not have a comment. I do not know what to add in these two places and why they are special. I also not sure that it is a deliberate choice, it can just be a copied behavior. Copy link Member vstinnerJan 16, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. As you want, I already approved the PR.
		return
Comment on lines +172 to +175 Copy link Contributor ronaldoussorenJan 2, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I'm slightly worried about the performance impact of calling stat on all`.pth` files.`pip install -e` will install a`.pth` file for every installed editable package. On the other hand, the overhead should be neglectable as the inode needs to be read in memory anyway to actually read the contents.
		if ((getattr(st, 'st_flags', 0) & stat.UF_HIDDEN) or
Copy link Contributor ronaldoussorenJan 2, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Which platforms support`UF_HIDDEN`? On macOS the flag is a hint that's ignored by command-line tools. E.g.: ronald@Pelargir[0]% touch foo.txt [~/Projects/Forks/cpython/build/X]ronald@Pelargir[0]% chflags hidden foo.txt [~/Projects/Forks/cpython/build/X]ronald@Pelargir[0]% ls -l (gh-53502-long-times)cpythontotal 0-rw-r--r-- 1 ronald staff 0 Jan 2 21:10 foo.txtronald@Pelargir[0]% ls -lO total 0-rw-r--r-- 1 ronald staff hidden 0 Jan 2 21:10 foo.txt This matches the documentation in`sys/stat.h`: #defineUF_HIDDEN 0x00008000/* hint that this item should not be // displayed in a GUI */
		(getattr(st, 'st_file_attributes', 0) & stat.FILE_ATTRIBUTE_HIDDEN)):
		_trace(f"Skipping hidden .pth file: {fullname!r}")
		return
		_trace(f"Processing .pth file: {fullname!r}")
		try:
		# locale encoding is not ideal especially on Windows. But we have used
Expand DownExpand Up		@@ -221,7 +230,8 @@ def addsitedir(sitedir, known_paths=None):
		names = os.listdir(sitedir)
		except OSError:
		return
		names = [name for name in names if name.endswith(".pth")]
		names = [name for name in names
		if name.endswith(".pth") and not name.startswith(".")]
		for name in sorted(names):
		addpackage(sitedir, name, known_paths)
		if reset:
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -194,6 +194,31 @@ def test_addsitedir(self):
		finally:
		pth_file.cleanup()

		def test_addsitedir_dotfile(self):
		pth_file = PthFile('.dotfile')
		pth_file.cleanup(prep=True)
		try:
		pth_file.create()
		site.addsitedir(pth_file.base_dir, set())
		self.assertNotIn(site.makepath(pth_file.good_dir_path)[0], sys.path)
Copy link Member vstinnerJan 10, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Maybe mock site.addpackage() and check that it's not called, rather than checking effects of site.addpackage() implementation. Same remark for the other added tests. Copy link MemberAuthor serhiy-storchakaJan 11, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. It is not called for dotfiles, but is called for files with hidden attribute. It is rather an implementation detail where we add a filter.
		self.assertIn(pth_file.base_dir, sys.path)
		finally:
		pth_file.cleanup()

		@unittest.skipUnless(hasattr(os, 'chflags'), 'test needs os.chflags()')
		def test_addsitedir_hidden_pth_file(self):
		pth_file = PthFile()
		pth_file.cleanup(prep=True)
		try:
		pth_file.create()
		st = os.stat(pth_file.file_path)
		os.chflags(target_file, st.st_flags \| stat.UF_IMMUTABLE)
		site.addsitedir(pth_file.base_dir, set())
		self.assertNotIn(site.makepath(pth_file.good_dir_path)[0], sys.path)
		self.assertIn(pth_file.base_dir, sys.path)
		finally:
		pth_file.cleanup()

		# This tests _getuserbase, hence the double underline
		# to distinguish from a test for getuserbase
		def test__getuserbase(self):
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Skip ``.pth`` files with names starting with a dot or hidden file attribute.