diff --git a/data/test/test_userfiles.py b/data/test/test_userfiles.py new file mode 100644 index 000000000..33e95b58f --- /dev/null +++ b/data/test/test_userfiles.py @@ -0,0 +1,15 @@ +import pytest + +from data.userfiles import DelegateUserfiles + +@pytest.mark.parametrize('path,expected', [ + ('foo', 'test/foo'), + ('bar', 'test/bar'), + ('/bar', 'test/bar'), + ('../foo', 'test/foo'), + ('foo/bar/baz', 'test/baz'), + ('foo/../baz', 'test/baz'), +]) +def test_filepath(path, expected): + userfiles = DelegateUserfiles(None, None, 'local_us', 'test') + assert userfiles.get_file_id_path(path) == expected diff --git a/data/userfiles.py b/data/userfiles.py index fb5e62fa7..c553658a5 100644 --- a/data/userfiles.py +++ b/data/userfiles.py @@ -32,6 +32,7 @@ class UserfilesHandlers(View): file_header_bytes = buffered.peek(1024) return send_file(buffered, mimetype=self._magic.from_buffer(file_header_bytes)) except IOError: + logger.exception('Error reading user file') abort(404) def put(self, file_id): @@ -73,7 +74,8 @@ class DelegateUserfiles(object): url_scheme=self._app.config['PREFERRED_URL_SCHEME']) def get_file_id_path(self, file_id): - return os.path.join(self._prefix, file_id) + # Note: We use basename here to prevent paths with ..'s and absolute paths. + return os.path.join(self._prefix, os.path.basename(file_id)) def prepare_for_drop(self, mime_type, requires_cors=True): """ Returns a signed URL to upload a file to our bucket. """