Merge pull request #1730 from geusebi/master

make safe_join behave like os.path.join with *args

CHANGES

@@ -9,6 +9,8 @@ Version 0.12
 - the cli command now responds to `--version`.
 - Mimetype guessing for ``send_file`` has been removed, as per issue ``#104``.
   See pull request ``#1849``.
+- Make ``flask.safe_join`` able to join multiple paths like ``os.path.join``
+  (pull request ``#1730``).
 
 Version 0.11.1
 --------------

flask/helpers.py

@@ -563,8 +563,9 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False,
     return rv
 
 
-def safe_join(directory, filename):
-    """Safely join `directory` and `filename`.
+def safe_join(directory, *pathnames):
+    """Safely join `directory` and zero or more untrusted `pathnames`
+    components.
 
     Example usage::
 
@@ -574,20 +575,23 @@ def wiki_page(filename):
             with open(filename, 'rb') as fd:
                 content = fd.read()  # Read and process the file content...
 
-    :param directory: the base directory.
-    :param filename: the untrusted filename relative to that directory.
-    :raises: :class:`~werkzeug.exceptions.NotFound` if the resulting path
-             would fall out of `directory`.
+    :param directory: the trusted base directory.
+    :param pathnames: the untrusted pathnames relative to that directory.
+    :raises: :class:`~werkzeug.exceptions.NotFound` if one or more passed
+            paths fall out of its boundaries.
     """
-    filename = posixpath.normpath(filename)
-    for sep in _os_alt_seps:
-        if sep in filename:
+    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('../'):
             raise NotFound()
-    if os.path.isabs(filename) or \
-       filename == '..' or \
-       filename.startswith('../'):
-        raise NotFound()
-    return os.path.join(directory, filename)
+        directory = os.path.join(directory, filename)
+    return directory
 
 
 def send_from_directory(directory, filename, **options):

tests/test_helpers.py

@@ -15,7 +15,7 @@
 import datetime
 import flask
 from logging import StreamHandler
-from werkzeug.exceptions import BadRequest
+from werkzeug.exceptions import BadRequest, NotFound
 from werkzeug.http import parse_cache_control_header, parse_options_header
 from werkzeug.http import http_date
 from flask._compat import StringIO, text_type
@@ -722,3 +722,45 @@ def generate():
         rv = c.get('/?name=World')
         assert rv.data == b'Hello World!'
         assert called == [42]
+
+
+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', ),
+            # If last path is '' add a slash
+            (('/a/b/c', '', ), '/a/b/c/', ),
+            # Preserve dot slash
+            (('/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'),
+            (('/..', ), '/..'),
+        )
+
+        for args, expected in passing:
+            assert flask.safe_join(*args) == expected
+
+    def test_safe_join_exceptions(self):
+        # Should raise werkzeug.exceptions.NotFound on unsafe joins.
+        failing = (
+            # path.isabs and ``..'' checks
+            ('/a', 'b', '/c'),
+            ('/a', '../b/c', ),
+            ('/a', '..', 'b/c'),
+            # Boundaries violations after path normalization
+            ('/a', 'b/../b/../../c', ),
+            ('/a', 'b', 'c/../..'),
+            ('/a', 'b/../../c', ),
+        )
+
+        for args in failing:
+            with pytest.raises(NotFound):
+                print(flask.safe_join(*args))