Merge pull request #2284 from davidism/windows-safe_join

safe_join on Windows uses posixpath

flask/helpers.py

@@ -638,18 +638,24 @@ def safe_join(directory, *pathnames):
     :raises: :class:`~werkzeug.exceptions.NotFound` if one or more passed
             paths fall out of its boundaries.
     """
+
+    parts = [directory]
+
     for filename in pathnames:
         if filename != '':
             filename = posixpath.normpath(filename)
-        for sep in _os_alt_seps:
-            if sep in filename:
-                raise NotFound()
-        if os.path.isabs(filename) or \
-           filename == '..' or \
-           filename.startswith('../'):
+
+        if (
+            any(sep in filename for sep in _os_alt_seps)
+            or os.path.isabs(filename)
+            or filename == '..'
+            or filename.startswith('../')
+        ):
             raise NotFound()
-        directory = os.path.join(directory, filename)
-    return directory
+
+        parts.append(filename)
+
+    return posixpath.join(*parts)
 
 
 def send_from_directory(directory, filename, **options):

tests/test_helpers.py

@@ -903,21 +903,20 @@ class TestStreaming(object):
 
 
 class TestSafeJoin(object):
-
     def test_safe_join(self):
         # Valid combinations of *args and expected joined paths.
         passing = (
-            (('a/b/c', ), 'a/b/c'),
-            (('/', 'a/', 'b/', 'c/', ), '/a/b/c'),
-            (('a', 'b', 'c', ), 'a/b/c'),
-            (('/a', 'b/c', ), '/a/b/c'),
-            (('a/b', 'X/../c'), 'a/b/c', ),
-            (('/a/b', 'c/X/..'), '/a/b/c', ),
+            (('a/b/c',), 'a/b/c'),
+            (('/', 'a/', 'b/', 'c/'), '/a/b/c'),
+            (('a', 'b', 'c'), 'a/b/c'),
+            (('/a', 'b/c'), '/a/b/c'),
+            (('a/b', 'X/../c'), 'a/b/c'),
+            (('/a/b', 'c/X/..'), '/a/b/c'),
             # If last path is '' add a slash
-            (('/a/b/c', '', ), '/a/b/c/', ),
+            (('/a/b/c', ''), '/a/b/c/'),
             # Preserve dot slash
-            (('/a/b/c', './', ), '/a/b/c/.', ),
-            (('a/b/c', 'X/..'), 'a/b/c/.', ),
+            (('/a/b/c', './'), '/a/b/c/.'),
+            (('a/b/c', 'X/..'), 'a/b/c/.'),
             # Base directory is always considered safe
             (('../', 'a/b/c'), '../a/b/c'),
             (('/..', ), '/..'),
@@ -931,12 +930,12 @@ class TestSafeJoin(object):
         failing = (
             # path.isabs and ``..'' checks
             ('/a', 'b', '/c'),
-            ('/a', '../b/c', ),
+            ('/a', '../b/c'),
             ('/a', '..', 'b/c'),
             # Boundaries violations after path normalization
-            ('/a', 'b/../b/../../c', ),
+            ('/a', 'b/../b/../../c'),
             ('/a', 'b', 'c/../..'),
-            ('/a', 'b/../../c', ),
+            ('/a', 'b/../../c'),
         )
 
         for args in failing: