From b110e6a478bbd35e05aaa0e0e4e13c6fb6166969 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Nov 2012 23:10:18 +0100 Subject: [PATCH] Move file path is in base dir checker to mopidy.utils.path --- mopidy/backends/local/stored_playlists.py | 28 ++++------------------- mopidy/utils/path.py | 19 +++++++++++++++ 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py index 5f9a18a1..04406c32 100644 --- a/mopidy/backends/local/stored_playlists.py +++ b/mopidy/backends/local/stored_playlists.py @@ -84,12 +84,12 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): def _get_m3u_path(self, name): name = formatting.slugify(name) file_path = os.path.join(self._path, name + '.m3u') - self._validate_file_path(file_path) + path.check_file_path_is_inside_base_dir(file_path, self._path) return file_path def _save_m3u(self, playlist): file_path = path.uri_to_path(playlist.uri) - self._validate_file_path(file_path) + path.check_file_path_is_inside_base_dir(file_path, self._path) with open(file_path, 'w') as file_handle: for track in playlist.tracks: if track.uri.startswith('file://'): @@ -100,35 +100,17 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): def _delete_m3u(self, uri): file_path = path.uri_to_path(uri) - self._validate_file_path(file_path) + path.check_file_path_is_inside_base_dir(file_path, self._path) if os.path.exists(file_path): os.remove(file_path) def _rename_m3u(self, playlist): src_file_path = path.uri_to_path(playlist.uri) - self._validate_file_path(src_file_path) + path.check_file_path_is_inside_base_dir(src_file_path, self._path) dst_file_path = self._get_m3u_path(playlist.name) - self._validate_file_path(dst_file_path) + path.check_file_path_is_inside_base_dir(dst_file_path, self._path) shutil.move(src_file_path, dst_file_path) return playlist.copy(uri=path.path_to_uri(dst_file_path)) - - def _validate_file_path(self, file_path): - assert not file_path.endswith(os.sep), ( - 'File path %s cannot end with a path separator' % file_path) - - # Expand symlinks - real_base_path = os.path.realpath(self._path) - real_file_path = os.path.realpath(file_path) - - # Use dir of file for prefix comparision, so we don't accept - # /tmp/foo.m3u as being inside /tmp/foo, simply because they have a - # common prefix, /tmp/foo, which matches the base path, /tmp/foo. - real_dir_path = os.path.dirname(real_file_path) - - # Check if dir of file is the base path or a subdir - common_prefix = os.path.commonprefix([real_base_path, real_dir_path]) - assert common_prefix == real_base_path, ( - 'File path %s must be in %s' % (real_file_path, real_base_path)) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index eef0c2db..1092534f 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -102,6 +102,25 @@ def find_files(path): yield filename +def check_file_path_is_inside_base_dir(file_path, base_path): + assert not file_path.endswith(os.sep), ( + 'File path %s cannot end with a path separator' % file_path) + + # Expand symlinks + real_base_path = os.path.realpath(base_path) + real_file_path = os.path.realpath(file_path) + + # Use dir of file for prefix comparision, so we don't accept + # /tmp/foo.m3u as being inside /tmp/foo, simply because they have a + # common prefix, /tmp/foo, which matches the base path, /tmp/foo. + real_dir_path = os.path.dirname(real_file_path) + + # Check if dir of file is the base path or a subdir + common_prefix = os.path.commonprefix([real_base_path, real_dir_path]) + assert common_prefix == real_base_path, ( + 'File path %s must be in %s' % (real_file_path, real_base_path)) + + # FIXME replace with mock usage in tests. class Mtime(object): def __init__(self):