Issue #6815: os.path.expandvars() now supports non-ASCII environment

variables names and values.

4 files changed, 93 insertions, 42 deletions

diff --git a/Lib/ntpath.py b/Lib/ntpath.py
index 5a012bdfb2..303e5862b3 100644
--- a/Lib/ntpath.py
+++ b/Lib/ntpath.py
@@ -362,6 +362,7 @@ def expandvars(path):
         percent = b'%'
         brace = b'{'
         dollar = b'$'
+        environ = getattr(os, 'environb', None)
     else:
         if '$' not in path and '%' not in path:
             return path
@@ -371,6 +372,7 @@ def expandvars(path):
         percent = '%'
         brace = '{'
         dollar = '$'
+        environ = os.environ
     res = path[:0]
     index = 0
     pathlen = len(path)
@@ -399,14 +401,13 @@ def expandvars(path):
                     index = pathlen - 1
                 else:
                     var = path[:index]
-                    if isinstance(path, bytes):
-                        var = var.decode('ascii')
-                    if var in os.environ:
-                        value = os.environ[var]
-                    else:
-                        value = '%' + var + '%'
-                    if isinstance(path, bytes):
-                        value = value.encode('ascii')
+                    try:
+                        if environ is None:
+                            value = os.fsencode(os.environ[os.fsdecode(var)])
+                        else:
+                            value = environ[var]
+                    except KeyError:
+                        value = percent + var + percent
                     res += value
         elif c == dollar:  # variable or '$$'
             if path[index + 1:index + 2] == dollar:
@@ -420,39 +421,40 @@ def expandvars(path):
                         index = path.index(b'}')
                     else:
                         index = path.index('}')
-                    var = path[:index]
-                    if isinstance(path, bytes):
-                        var = var.decode('ascii')
-                    if var in os.environ:
-                        value = os.environ[var]
-                    else:
-                        value = '${' + var + '}'
-                    if isinstance(path, bytes):
-                        value = value.encode('ascii')
-                    res += value
                 except ValueError:
                     if isinstance(path, bytes):
                         res += b'${' + path
                     else:
                         res += '${' + path
                     index = pathlen - 1
+                else:
+                    var = path[:index]
+                    try:
+                        if environ is None:
+                            value = os.fsencode(os.environ[os.fsdecode(var)])
+                        else:
+                            value = environ[var]
+                    except KeyError:
+                        if isinstance(path, bytes):
+                            value = b'${' + var + b'}'
+                        else:
+                            value = '${' + var + '}'
+                    res += value
             else:
-                var = ''
+                var = path[:0]
                 index += 1
                 c = path[index:index + 1]
                 while c and c in varchars:
-                    if isinstance(path, bytes):
-                        var += c.decode('ascii')
-                    else:
-                        var += c
+                    var += c
                     index += 1
                     c = path[index:index + 1]
-                if var in os.environ:
-                    value = os.environ[var]
-                else:
-                    value = '$' + var
-                if isinstance(path, bytes):
-                    value = value.encode('ascii')
+                try:
+                    if environ is None:
+                        value = os.fsencode(os.environ[os.fsdecode(var)])
+                    else:
+                        value = environ[var]
+                except KeyError:
+                    value = dollar + var
                 res += value
                 if c:
                     index -= 1
diff --git a/Lib/posixpath.py b/Lib/posixpath.py
index b1e1a9255e..9c11d8a32b 100644
--- a/Lib/posixpath.py
+++ b/Lib/posixpath.py
@@ -300,6 +300,7 @@ def expandvars(path):
         search = _varprogb.search
         start = b'{'
         end = b'}'
+        environ = getattr(os, 'environb', None)
     else:
         if '$' not in path:
             return path
@@ -309,6 +310,7 @@ def expandvars(path):
         search = _varprog.search
         start = '{'
         end = '}'
+        environ = os.environ
     i = 0
     while True:
         m = search(path, i)
@@ -318,18 +320,18 @@ def expandvars(path):
         name = m.group(1)
         if name.startswith(start) and name.endswith(end):
             name = name[1:-1]
-        if isinstance(name, bytes):
-            name = str(name, 'ASCII')
-        if name in os.environ:
+        try:
+            if environ is None:
+                value = os.fsencode(os.environ[os.fsdecode(var)])
+            else:
+                value = environ[name]
+        except KeyError:
+            i = j
+        else:
             tail = path[j:]
-            value = os.environ[name]
-            if isinstance(path, bytes):
-                value = value.encode('ASCII')
             path = path[:i] + value
             i = len(path)
             path += tail
-        else:
-            i = j
     return path
 
 
diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py
index fd8bc577ca..b5068e4c98 100644
--- a/Lib/test/test_genericpath.py
+++ b/Lib/test/test_genericpath.py
@@ -248,7 +248,6 @@ class CommonTest(GenericTest):
             self.assertEqual(expandvars("$[foo]bar"), "$[foo]bar")
             self.assertEqual(expandvars("$bar bar"), "$bar bar")
             self.assertEqual(expandvars("$?bar"), "$?bar")
-            self.assertEqual(expandvars("${foo}bar"), "barbar")
             self.assertEqual(expandvars("$foo}bar"), "bar}bar")
             self.assertEqual(expandvars("${foo"), "${foo")
             self.assertEqual(expandvars("${{foo}}"), "baz1}")
@@ -261,13 +260,40 @@ class CommonTest(GenericTest):
             self.assertEqual(expandvars(b"$[foo]bar"), b"$[foo]bar")
             self.assertEqual(expandvars(b"$bar bar"), b"$bar bar")
             self.assertEqual(expandvars(b"$?bar"), b"$?bar")
-            self.assertEqual(expandvars(b"${foo}bar"), b"barbar")
             self.assertEqual(expandvars(b"$foo}bar"), b"bar}bar")
             self.assertEqual(expandvars(b"${foo"), b"${foo")
             self.assertEqual(expandvars(b"${{foo}}"), b"baz1}")
             self.assertEqual(expandvars(b"$foo$foo"), b"barbar")
             self.assertEqual(expandvars(b"$bar$bar"), b"$bar$bar")
 
+    @unittest.skipUnless(support.FS_NONASCII, 'need support.FS_NONASCII')
+    def test_expandvars_nonascii(self):
+        if self.pathmodule.__name__ == 'macpath':
+            self.skipTest('macpath.expandvars is a stub')
+        expandvars = self.pathmodule.expandvars
+        def check(value, expected):
+            self.assertEqual(expandvars(value), expected)
+        with support.EnvironmentVarGuard() as env:
+            env.clear()
+            nonascii = support.FS_NONASCII
+            env['spam'] = nonascii
+            env[nonascii] = 'ham' + nonascii
+            check(nonascii, nonascii)
+            check('$spam bar', '%s bar' % nonascii)
+            check('${spam}bar', '%sbar' % nonascii)
+            check('${%s}bar' % nonascii, 'ham%sbar' % nonascii)
+            check('$bar%s bar' % nonascii, '$bar%s bar' % nonascii)
+            check('$spam}bar', '%s}bar' % nonascii)
+
+            check(os.fsencode(nonascii), os.fsencode(nonascii))
+            check(b'$spam bar', os.fsencode('%s bar' % nonascii))
+            check(b'${spam}bar', os.fsencode('%sbar' % nonascii))
+            check(os.fsencode('${%s}bar' % nonascii),
+                  os.fsencode('ham%sbar' % nonascii))
+            check(os.fsencode('$bar%s bar' % nonascii),
+                  os.fsencode('$bar%s bar' % nonascii))
+            check(b'$spam}bar', os.fsencode('%s}bar' % nonascii))
+
     def test_abspath(self):
         self.assertIn("foo", self.pathmodule.abspath("foo"))
         with warnings.catch_warnings():
diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py
index 2c4d27e6f5..e9e1d715ef 100644
--- a/Lib/test/test_ntpath.py
+++ b/Lib/test/test_ntpath.py
@@ -22,13 +22,15 @@ def tester(fn, wantResult):
     fn = fn.replace('["', '[b"')
     fn = fn.replace(", '", ", b'")
     fn = fn.replace(', "', ', b"')
+    fn = os.fsencode(fn).decode('latin1')
+    fn = fn.encode('ascii', 'backslashreplace').decode('ascii')
     with warnings.catch_warnings():
         warnings.simplefilter("ignore", DeprecationWarning)
         gotResult = eval(fn)
     if isinstance(wantResult, str):
-        wantResult = wantResult.encode('ascii')
+        wantResult = os.fsencode(wantResult)
     elif isinstance(wantResult, tuple):
-        wantResult = tuple(r.encode('ascii') for r in wantResult)
+        wantResult = tuple(os.fsencode(r) for r in wantResult)
 
     gotResult = eval(fn)
     if wantResult != gotResult:
@@ -223,7 +225,6 @@ class TestNtpath(unittest.TestCase):
             tester('ntpath.expandvars("$[foo]bar")', "$[foo]bar")
             tester('ntpath.expandvars("$bar bar")', "$bar bar")
             tester('ntpath.expandvars("$?bar")', "$?bar")
-            tester('ntpath.expandvars("${foo}bar")', "barbar")
             tester('ntpath.expandvars("$foo}bar")', "bar}bar")
             tester('ntpath.expandvars("${foo")', "${foo")
             tester('ntpath.expandvars("${{foo}}")', "baz1}")
@@ -237,6 +238,26 @@ class TestNtpath(unittest.TestCase):
             tester('ntpath.expandvars("%foo%%bar")', "bar%bar")
             tester('ntpath.expandvars("\'%foo%\'%bar")', "\'%foo%\'%bar")
 
+    @unittest.skipUnless(support.FS_NONASCII, 'need support.FS_NONASCII')
+    def test_expandvars_nonascii(self):
+        def check(value, expected):
+            tester('ntpath.expandvars(%r)' % value, expected)
+        with support.EnvironmentVarGuard() as env:
+            env.clear()
+            nonascii = support.FS_NONASCII
+            env['spam'] = nonascii
+            env[nonascii] = 'ham' + nonascii
+            check('$spam bar', '%s bar' % nonascii)
+            check('$%s bar' % nonascii, '$%s bar' % nonascii)
+            check('${spam}bar', '%sbar' % nonascii)
+            check('${%s}bar' % nonascii, 'ham%sbar' % nonascii)
+            check('$spam}bar', '%s}bar' % nonascii)
+            check('$%s}bar' % nonascii, '$%s}bar' % nonascii)
+            check('%spam% bar', '%s bar' % nonascii)
+            check('%{}% bar'.format(nonascii), 'ham%s bar' % nonascii)
+            check('%spam%bar', '%sbar' % nonascii)
+            check('%{}%bar'.format(nonascii), 'ham%sbar' % nonascii)
+
     def test_abspath(self):
         # ntpath.abspath() can only be used on a system with the "nt" module
         # (reasonably), so we protect this test with "import nt".  This allows