From f7c35bf0d51d1ae02709e39fe29110e12f64fb87 Mon Sep 17 00:00:00 2001 From: David Lord Date: Mon, 15 May 2017 16:58:01 -0700 Subject: [PATCH] safe_join on Windows uses posixpath fixes #2033 closes #2059 --- CHANGES | 7 +++++++ flask/helpers.py | 22 ++++++++++++++-------- tests/test_helpers.py | 25 ++++++++++++------------- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/CHANGES b/CHANGES index 613b8189..a67c8bf9 100644 --- a/CHANGES +++ b/CHANGES @@ -15,6 +15,13 @@ Major release, unreleased method returns compressed response by default, and pretty response in debug mode. +Version 0.12.2 +-------------- + +Bugfix release + +- Fix a bug in `safe_join` on Windows. + Version 0.12.1 -------------- diff --git a/flask/helpers.py b/flask/helpers.py index c6c2cddc..4bb1d1c9 100644 --- a/flask/helpers.py +++ b/flask/helpers.py @@ -619,18 +619,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): diff --git a/tests/test_helpers.py b/tests/test_helpers.py index b1241ce9..9320ef71 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -846,21 +846,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'), (('/..', ), '/..'), @@ -874,12 +873,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: