bpo-10496: posixpath.expanduser() catchs pwd.getpwuid() error (GH-10919)

miss-islington · vstinner · web-flow · commit 983d1ab0e6f4 · 2018-12-05T08:07:57.000-08:00
* posixpath.expanduser() now returns the input path unchanged if the HOME environment variable is not set and pwd.getpwuid() raises KeyError (the current user identifier doesn't exist in the password database). * Add test_no_home_directory() to test_site. (cherry picked from commit f2f4555) Co-authored-by: Victor Stinner <vstinner@redhat.com>
diff --git a/Lib/posixpath.py b/Lib/posixpath.py
@@ -246,7 +246,12 @@ def expanduser(path):
     if i == 1:
         if 'HOME' not in os.environ:
             import pwd
-            userhome = pwd.getpwuid(os.getuid()).pw_dir
+            try:
+                userhome = pwd.getpwuid(os.getuid()).pw_dir
+            except KeyError:
+                # bpo-10496: if the current user identifier doesn't exist in the
+                # password database, return the path unchanged
+                return path
         else:
             userhome = os.environ['HOME']
     else:
@@ -257,6 +262,8 @@ def expanduser(path):
         try:
             pwent = pwd.getpwnam(name)
         except KeyError:
+            # bpo-10496: if the user name from the path doesn't exist in the
+            # password database, return the path unchanged
             return path
         userhome = pwent.pw_dir
     if isinstance(path, bytes):
diff --git a/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py
@@ -5,6 +5,7 @@
 from posixpath import realpath, abspath, dirname, basename
 from test import support, test_genericpath
 from test.support import FakePath
+from unittest import mock
 
 try:
     import posix
@@ -230,42 +231,61 @@ def fake_lstat(path):
     def test_expanduser(self):
         self.assertEqual(posixpath.expanduser("foo"), "foo")
         self.assertEqual(posixpath.expanduser(b"foo"), b"foo")
+
+    def test_expanduser_home_envvar(self):
         with support.EnvironmentVarGuard() as env:
+            env['HOME'] = '/home/victor'
+            self.assertEqual(posixpath.expanduser("~"), "/home/victor")
+
+            # expanduser() strips trailing slash
+            env['HOME'] = '/home/victor/'
+            self.assertEqual(posixpath.expanduser("~"), "/home/victor")
+
             for home in '/', '', '//', '///':
                 with self.subTest(home=home):
                     env['HOME'] = home
                     self.assertEqual(posixpath.expanduser("~"), "/")
                     self.assertEqual(posixpath.expanduser("~/"), "/")
                     self.assertEqual(posixpath.expanduser("~/foo"), "/foo")
-        try:
-            import pwd
-        except ImportError:
-            pass
-        else:
-            self.assertIsInstance(posixpath.expanduser("~/"), str)
-            self.assertIsInstance(posixpath.expanduser(b"~/"), bytes)
-            # if home directory == root directory, this test makes no sense
-            if posixpath.expanduser("~") != '/':
-                self.assertEqual(
-                    posixpath.expanduser("~") + "/",
-                    posixpath.expanduser("~/")
-                )
-                self.assertEqual(
-                    posixpath.expanduser(b"~") + b"/",
-                    posixpath.expanduser(b"~/")
-                )
-            self.assertIsInstance(posixpath.expanduser("~root/"), str)
-            self.assertIsInstance(posixpath.expanduser("~foo/"), str)
-            self.assertIsInstance(posixpath.expanduser(b"~root/"), bytes)
-            self.assertIsInstance(posixpath.expanduser(b"~foo/"), bytes)
-
-            with support.EnvironmentVarGuard() as env:
-                # expanduser should fall back to using the password database
-                del env['HOME']
-                home = pwd.getpwuid(os.getuid()).pw_dir
-                # $HOME can end with a trailing /, so strip it (see #17809)
-                home = home.rstrip("/") or '/'
-                self.assertEqual(posixpath.expanduser("~"), home)
+
+    def test_expanduser_pwd(self):
+        pwd = support.import_module('pwd')
+
+        self.assertIsInstance(posixpath.expanduser("~/"), str)
+        self.assertIsInstance(posixpath.expanduser(b"~/"), bytes)
+
+        # if home directory == root directory, this test makes no sense
+        if posixpath.expanduser("~") != '/':
+            self.assertEqual(
+                posixpath.expanduser("~") + "/",
+                posixpath.expanduser("~/")
+            )
+            self.assertEqual(
+                posixpath.expanduser(b"~") + b"/",
+                posixpath.expanduser(b"~/")
+            )
+        self.assertIsInstance(posixpath.expanduser("~root/"), str)
+        self.assertIsInstance(posixpath.expanduser("~foo/"), str)
+        self.assertIsInstance(posixpath.expanduser(b"~root/"), bytes)
+        self.assertIsInstance(posixpath.expanduser(b"~foo/"), bytes)
+
+        with support.EnvironmentVarGuard() as env:
+            # expanduser should fall back to using the password database
+            del env['HOME']
+
+            home = pwd.getpwuid(os.getuid()).pw_dir
+            # $HOME can end with a trailing /, so strip it (see #17809)
+            home = home.rstrip("/") or '/'
+            self.assertEqual(posixpath.expanduser("~"), home)
+
+            # bpo-10496: If the HOME environment variable is not set and the
+            # user (current identifier or name in the path) doesn't exist in
+            # the password database (pwd.getuid() or pwd.getpwnam() fail),
+            # expanduser() must return the path unchanged.
+            with mock.patch.object(pwd, 'getpwuid', side_effect=KeyError), \
+                 mock.patch.object(pwd, 'getpwnam', side_effect=KeyError):
+                for path in ('~', '~/.local', '~vstinner/'):
+                    self.assertEqual(posixpath.expanduser(path), path)
 
     def test_normpath(self):
         self.assertEqual(posixpath.normpath(""), ".")
diff --git a/Lib/test/test_site.py b/Lib/test/test_site.py
@@ -6,6 +6,7 @@
 """
 import unittest
 import test.support
+from test import support
 from test.support import (captured_stderr, TESTFN, EnvironmentVarGuard,
                           change_cwd)
 import builtins
@@ -19,6 +20,7 @@
 import subprocess
 import sysconfig
 import tempfile
+from unittest import mock
 from copy import copy
 
 # These tests are not particularly useful if Python was invoked with -S.
@@ -258,6 +260,7 @@ def test_getusersitepackages(self):
         # the call sets USER_BASE *and* USER_SITE
         self.assertEqual(site.USER_SITE, user_site)
         self.assertTrue(user_site.startswith(site.USER_BASE), user_site)
+        self.assertEqual(site.USER_BASE, site.getuserbase())
 
     def test_getsitepackages(self):
         site.PREFIXES = ['xoxo']
@@ -276,6 +279,40 @@ def test_getsitepackages(self):
             wanted = os.path.join('xoxo', 'lib', 'site-packages')
             self.assertEqual(dirs[1], wanted)
 
+    def test_no_home_directory(self):
+        # bpo-10496: getuserbase() and getusersitepackages() must not fail if
+        # the current user has no home directory (if expanduser() returns the
+        # path unchanged).
+        site.USER_SITE = None
+        site.USER_BASE = None
+
+        with EnvironmentVarGuard() as environ, \
+             mock.patch('os.path.expanduser', lambda path: path):
+
+            del environ['PYTHONUSERBASE']
+            del environ['APPDATA']
+
+            user_base = site.getuserbase()
+            self.assertTrue(user_base.startswith('~' + os.sep),
+                            user_base)
+
+            user_site = site.getusersitepackages()
+            self.assertTrue(user_site.startswith(user_base), user_site)
+
+        with mock.patch('os.path.isdir', return_value=False) as mock_isdir, \
+             mock.patch.object(site, 'addsitedir') as mock_addsitedir, \
+             support.swap_attr(site, 'ENABLE_USER_SITE', True):
+
+            # addusersitepackages() must not add user_site to sys.path
+            # if it is not an existing directory
+            known_paths = set()
+            site.addusersitepackages(known_paths)
+
+            mock_isdir.assert_called_once_with(user_site)
+            mock_addsitedir.assert_not_called()
+            self.assertFalse(known_paths)
+
+
 class PthFile(object):
     """Helper class for handling testing of .pth files"""
 
diff --git a/Misc/NEWS.d/next/Library/2018-12-05-13-37-39.bpo-10496.VH-1Lp.rst b/Misc/NEWS.d/next/Library/2018-12-05-13-37-39.bpo-10496.VH-1Lp.rst
@@ -0,0 +1,5 @@
+:func:`posixpath.expanduser` now returns the input *path* unchanged if the
+``HOME`` environment variable is not set and the current user has no home
+directory (if the current user identifier doesn't exist in the password
+database). This change fix the :mod:`site` module if the current user doesn't
+exist in the password database (if the user has no home directory).
