diff options
author | Max Kellermann <max@duempel.org> | 2014-02-02 14:37:52 +0100 |
---|---|---|
committer | Max Kellermann <max@duempel.org> | 2014-02-03 23:32:10 +0100 |
commit | ca36ac2ba196ee2bbe4b54ee9a71d49174803277 (patch) | |
tree | d365b1ac7872e1785befdcebf254885c1c27a268 /src | |
parent | ba675d6a55769a6e82a6efaa2f4a812a4eea2362 (diff) | |
download | mpd-ca36ac2ba196ee2bbe4b54ee9a71d49174803277.tar.gz mpd-ca36ac2ba196ee2bbe4b54ee9a71d49174803277.tar.xz mpd-ca36ac2ba196ee2bbe4b54ee9a71d49174803277.zip |
SongLoader: new class that merges duplicate code
There was quite a lot of duplicate code for loading DetachedSong
objects, with different semantics for "securely" loading local files.
Diffstat (limited to 'src')
-rw-r--r-- | src/Partition.hxx | 11 | ||||
-rw-r--r-- | src/Playlist.hxx | 12 | ||||
-rw-r--r-- | src/PlaylistEdit.cxx | 34 | ||||
-rw-r--r-- | src/PlaylistSave.cxx | 14 | ||||
-rw-r--r-- | src/SongLoader.cxx | 102 | ||||
-rw-r--r-- | src/SongLoader.hxx | 52 | ||||
-rw-r--r-- | src/command/PlaylistCommands.cxx | 4 | ||||
-rw-r--r-- | src/command/QueueCommands.cxx | 85 | ||||
-rw-r--r-- | src/playlist/PlaylistQueue.cxx | 8 | ||||
-rw-r--r-- | src/playlist/PlaylistQueue.hxx | 5 | ||||
-rw-r--r-- | src/playlist/PlaylistSong.cxx | 67 | ||||
-rw-r--r-- | src/playlist/PlaylistSong.hxx | 5 | ||||
-rw-r--r-- | src/playlist/Print.cxx | 5 |
13 files changed, 235 insertions, 169 deletions
diff --git a/src/Partition.hxx b/src/Partition.hxx index de41162f3..57dc9a5f4 100644 --- a/src/Partition.hxx +++ b/src/Partition.hxx @@ -26,6 +26,7 @@ struct Instance; class MultipleOutputs; +class SongLoader; /** * A partition of the Music Player Daemon. It is a separate unit with @@ -51,14 +52,10 @@ struct Partition { playlist.Clear(pc); } - PlaylistResult AppendFile(const char *path_utf8, - unsigned *added_id=nullptr) { - return playlist.AppendFile(pc, path_utf8, added_id); - } - - PlaylistResult AppendURI(const char *uri_utf8, + PlaylistResult AppendURI(const SongLoader &loader, + const char *uri_utf8, unsigned *added_id=nullptr) { - return playlist.AppendURI(pc, uri_utf8, added_id); + return playlist.AppendURI(pc, loader, uri_utf8, added_id); } PlaylistResult DeletePosition(unsigned position) { diff --git a/src/Playlist.hxx b/src/Playlist.hxx index 45f1e2dfc..09980155e 100644 --- a/src/Playlist.hxx +++ b/src/Playlist.hxx @@ -28,6 +28,7 @@ struct PlayerControl; class DetachedSong; class Database; class Error; +class SongLoader; struct playlist { /** @@ -149,17 +150,8 @@ public: DetachedSong &&song, unsigned *added_id=nullptr); - /** - * Appends a local file (outside the music database) to the - * playlist. - * - * Note: the caller is responsible for checking permissions. - */ - PlaylistResult AppendFile(PlayerControl &pc, - const char *path_utf8, - unsigned *added_id=nullptr); - PlaylistResult AppendURI(PlayerControl &pc, + const SongLoader &loader, const char *uri_utf8, unsigned *added_id=nullptr); diff --git a/src/PlaylistEdit.cxx b/src/PlaylistEdit.cxx index 11b867546..8d2c76e6e 100644 --- a/src/PlaylistEdit.cxx +++ b/src/PlaylistEdit.cxx @@ -30,9 +30,8 @@ #include "util/UriUtil.hxx" #include "util/Error.hxx" #include "DetachedSong.hxx" -#include "Mapper.hxx" +#include "SongLoader.hxx" #include "Idle.hxx" -#include "db/DatabaseSong.hxx" #include "Log.hxx" #include <stdlib.h> @@ -57,17 +56,6 @@ playlist::Clear(PlayerControl &pc) } PlaylistResult -playlist::AppendFile(PlayerControl &pc, - const char *path_utf8, unsigned *added_id) -{ - DetachedSong song(path_utf8); - if (!song.Update()) - return PlaylistResult::NO_SUCH_SONG; - - return AppendSong(pc, std::move(song), added_id); -} - -PlaylistResult playlist::AppendSong(PlayerControl &pc, DetachedSong &&song, unsigned *added_id) { @@ -81,7 +69,7 @@ playlist::AppendSong(PlayerControl &pc, id = queue.Append(std::move(song), 0); if (queue.random) { - /* shuffle the new song into the list of remaining + /* shuffle the new song into the list of remaning songs to play */ unsigned start; @@ -104,19 +92,19 @@ playlist::AppendSong(PlayerControl &pc, PlaylistResult playlist::AppendURI(PlayerControl &pc, + const SongLoader &loader, const char *uri, unsigned *added_id) { FormatDebug(playlist_domain, "add to playlist: %s", uri); - DetachedSong *song; - if (uri_has_scheme(uri)) { - song = new DetachedSong(uri); - } else { -#ifdef ENABLE_DATABASE - song = DatabaseDetachSong(uri, IgnoreError()); - if (song == nullptr) -#endif - return PlaylistResult::NO_SUCH_SONG; + Error error; + DetachedSong *song = loader.LoadSong(uri, error); + if (song == nullptr) { + // TODO: return the Error + LogError(error); + return error.IsDomain(playlist_domain) + ? PlaylistResult(error.GetCode()) + : PlaylistResult::NO_SUCH_SONG; } PlaylistResult result = AppendSong(pc, std::move(*song), added_id); diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx index f2541e71b..d3369c9b6 100644 --- a/src/PlaylistSave.cxx +++ b/src/PlaylistSave.cxx @@ -23,6 +23,7 @@ #include "PlaylistError.hxx" #include "Playlist.hxx" #include "DetachedSong.hxx" +#include "SongLoader.hxx" #include "Mapper.hxx" #include "Idle.hxx" #include "fs/AllocatedPath.hxx" @@ -117,19 +118,12 @@ playlist_load_spl(struct playlist &playlist, PlayerControl &pc, if (end_index > contents.size()) end_index = contents.size(); + const SongLoader loader(nullptr); + for (unsigned i = start_index; i < end_index; ++i) { const auto &uri_utf8 = contents[i]; - if (memcmp(uri_utf8.c_str(), "file:///", 8) == 0) { - const char *path_utf8 = uri_utf8.c_str() + 7; - - if (playlist.AppendFile(pc, path_utf8) != PlaylistResult::SUCCESS) - FormatError(playlist_domain, - "can't add file \"%s\"", path_utf8); - continue; - } - - if ((playlist.AppendURI(pc, uri_utf8.c_str())) != PlaylistResult::SUCCESS) + if ((playlist.AppendURI(pc, loader, uri_utf8.c_str())) != PlaylistResult::SUCCESS) FormatError(playlist_domain, "can't add file \"%s\"", uri_utf8.c_str()); } diff --git a/src/SongLoader.cxx b/src/SongLoader.cxx new file mode 100644 index 000000000..2ed34671c --- /dev/null +++ b/src/SongLoader.cxx @@ -0,0 +1,102 @@ +/* + * Copyright (C) 2003-2014 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "config.h" +#include "SongLoader.hxx" +#include "client/Client.hxx" +#include "Mapper.hxx" +#include "db/DatabaseSong.hxx" +#include "ls.hxx" +#include "fs/AllocatedPath.hxx" +#include "fs/Traits.hxx" +#include "util/UriUtil.hxx" +#include "util/Error.hxx" +#include "DetachedSong.hxx" +#include "PlaylistError.hxx" + +#include <assert.h> +#include <string.h> + +DetachedSong * +SongLoader::LoadFile(const char *path_utf8, Error &error) const +{ +#ifdef ENABLE_DATABASE + /* TODO fs_charset vs utf8? */ + const char *suffix = map_to_relative_path(path_utf8); + assert(suffix != nullptr); + + if (suffix != path_utf8) + /* this path was relative to the music directory - + obtain it from the database */ + return LoadSong(suffix, error); +#endif + + if (client != nullptr) { + const auto path_fs = AllocatedPath::FromUTF8(path_utf8, error); + if (path_fs.IsNull()) + return nullptr; + + if (!client->AllowFile(path_fs, error)) + return nullptr; + } + + DetachedSong *song = new DetachedSong(path_utf8); + if (!song->Update()) { + error.Set(playlist_domain, int(PlaylistResult::NO_SUCH_SONG), + "No such file"); + delete song; + return nullptr; + } + + return song; +} + +DetachedSong * +SongLoader::LoadSong(const char *uri_utf8, Error &error) const +{ + assert(uri_utf8 != nullptr); + + if (memcmp(uri_utf8, "file:///", 8) == 0) + /* absolute path */ + return LoadFile(uri_utf8 + 7, error); + else if (PathTraitsUTF8::IsAbsolute(uri_utf8)) + /* absolute path */ + return LoadFile(uri_utf8, error); + else if (uri_has_scheme(uri_utf8)) { + /* remove URI */ + if (!uri_supported_scheme(uri_utf8)) { + error.Set(playlist_domain, + int(PlaylistResult::NO_SUCH_SONG), + "Unsupported URI scheme"); + return nullptr; + } + + return new DetachedSong(uri_utf8); + } else { + /* URI relative to the music directory */ + +#ifdef ENABLE_DATABASE + return DatabaseDetachSong(uri_utf8, error); +#else + error.Set(playlist_domain, int(PlaylistResult::NO_SUCH_SONG), + "No database"); + return nullptr; +#endif + } +} diff --git a/src/SongLoader.hxx b/src/SongLoader.hxx new file mode 100644 index 000000000..aaa8060db --- /dev/null +++ b/src/SongLoader.hxx @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2003-2014 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef MPD_SONG_LOADER_HXX +#define MPD_SONG_LOADER_HXX + +#include "Compiler.h" + +class Client; +class DetachedSong; +class Error; + +/** + * A utility class that loads a #DetachedSong object by its URI. If + * the URI is an absolute local file, it applies security checks via + * Client::AllowFile(). If no #Client pointer was specified, then it + * is assumed that all local files are allowed. + */ +class SongLoader { + const Client *const client; + +public: + explicit SongLoader(const Client &_client) + :client(&_client) {} + explicit SongLoader(const Client *_client) + :client(_client) {} + + gcc_nonnull_all + DetachedSong *LoadSong(const char *uri_utf8, Error &error) const; + +private: + gcc_nonnull_all + DetachedSong *LoadFile(const char *path_utf8, Error &error) const; +}; + +#endif diff --git a/src/command/PlaylistCommands.cxx b/src/command/PlaylistCommands.cxx index 272fb0ee4..c3c4fe37c 100644 --- a/src/command/PlaylistCommands.cxx +++ b/src/command/PlaylistCommands.cxx @@ -25,6 +25,7 @@ #include "PlaylistSave.hxx" #include "PlaylistFile.hxx" #include "db/PlaylistVector.hxx" +#include "SongLoader.hxx" #include "playlist/PlaylistQueue.hxx" #include "playlist/Print.hxx" #include "TimePrint.hxx" @@ -65,11 +66,12 @@ handle_load(Client &client, int argc, char *argv[]) } else if (!check_range(client, &start_index, &end_index, argv[2])) return CommandResult::ERROR; + const SongLoader loader(client); const PlaylistResult result = playlist_open_into_queue(argv[1], start_index, end_index, client.playlist, - client.player_control, true); + client.player_control, loader); if (result != PlaylistResult::NO_SUCH_LIST) return print_playlist_result(client, result); diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx index 5bef1b461..f14beb872 100644 --- a/src/command/QueueCommands.cxx +++ b/src/command/QueueCommands.cxx @@ -21,8 +21,9 @@ #include "QueueCommands.hxx" #include "CommandError.hxx" #include "db/DatabaseQueue.hxx" -#include "SongFilter.hxx" #include "db/Selection.hxx" +#include "SongFilter.hxx" +#include "SongLoader.hxx" #include "Playlist.hxx" #include "PlaylistPrint.hxx" #include "client/Client.hxx" @@ -38,38 +39,32 @@ #include <string.h> -CommandResult -handle_add(Client &client, gcc_unused int argc, char *argv[]) +static const char * +translate_uri(Client &client, const char *uri) { - char *uri = argv[1]; - PlaylistResult result; - - if (memcmp(uri, "file:///", 8) == 0) { - const char *path_utf8 = uri + 7; - const auto path_fs = AllocatedPath::FromUTF8(path_utf8); - - if (path_fs.IsNull()) { - command_error(client, ACK_ERROR_NO_EXIST, - "unsupported file name"); - return CommandResult::ERROR; - } - - Error error; - if (!client.AllowFile(path_fs, error)) - return print_error(client, error); - - result = client.partition.AppendFile(path_utf8); - return print_playlist_result(client, result); + if (memcmp(uri, "file:///", 8) == 0) + /* drop the "file://", leave only an absolute path + (starting with a slash) */ + return uri + 7; + + if (PathTraitsUTF8::IsAbsolute(uri)) { + command_error(client, ACK_ERROR_NO_EXIST, "Malformed URI"); + return nullptr; } - if (uri_has_scheme(uri)) { - if (!uri_supported_scheme(uri)) { - command_error(client, ACK_ERROR_NO_EXIST, - "unsupported URI scheme"); - return CommandResult::ERROR; - } + return uri; +} - result = client.partition.AppendURI(uri); +CommandResult +handle_add(Client &client, gcc_unused int argc, char *argv[]) +{ + const char *const uri = translate_uri(client, argv[1]); + if (uri == nullptr) + return CommandResult::ERROR; + + if (uri_has_scheme(uri) || PathTraitsUTF8::IsAbsolute(uri)) { + const SongLoader loader(client); + auto result = client.partition.AppendURI(loader, uri); return print_playlist_result(client, result); } @@ -88,34 +83,14 @@ handle_add(Client &client, gcc_unused int argc, char *argv[]) CommandResult handle_addid(Client &client, int argc, char *argv[]) { - char *uri = argv[1]; - unsigned added_id; - PlaylistResult result; - - if (memcmp(uri, "file:///", 8) == 0) { - const char *path_utf8 = uri + 7; - const auto path_fs = AllocatedPath::FromUTF8(path_utf8); - - if (path_fs.IsNull()) { - command_error(client, ACK_ERROR_NO_EXIST, - "unsupported file name"); - return CommandResult::ERROR; - } + const char *const uri = translate_uri(client, argv[1]); + if (uri == nullptr) + return CommandResult::ERROR; - Error error; - if (!client.AllowFile(path_fs, error)) - return print_error(client, error); + const SongLoader loader(client); - result = client.partition.AppendFile(path_utf8, &added_id); - } else { - if (uri_has_scheme(uri) && !uri_supported_scheme(uri)) { - command_error(client, ACK_ERROR_NO_EXIST, - "unsupported URI scheme"); - return CommandResult::ERROR; - } - - result = client.partition.AppendURI(uri, &added_id); - } + unsigned added_id; + auto result = client.partition.AppendURI(loader, uri, &added_id); if (result != PlaylistResult::SUCCESS) return print_playlist_result(client, result); diff --git a/src/playlist/PlaylistQueue.cxx b/src/playlist/PlaylistQueue.cxx index 564c94d0a..9adea317f 100644 --- a/src/playlist/PlaylistQueue.cxx +++ b/src/playlist/PlaylistQueue.cxx @@ -32,7 +32,7 @@ PlaylistResult playlist_load_into_queue(const char *uri, SongEnumerator &e, unsigned start_index, unsigned end_index, playlist &dest, PlayerControl &pc, - bool secure) + const SongLoader &loader) { const std::string base_uri = uri != nullptr ? PathTraitsUTF8::GetParent(uri) @@ -49,7 +49,7 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e, } if (!playlist_check_translate_song(*song, base_uri.c_str(), - secure)) { + loader)) { delete song; continue; } @@ -67,7 +67,7 @@ PlaylistResult playlist_open_into_queue(const char *uri, unsigned start_index, unsigned end_index, playlist &dest, PlayerControl &pc, - bool secure) + const SongLoader &loader) { Mutex mutex; Cond cond; @@ -80,7 +80,7 @@ playlist_open_into_queue(const char *uri, PlaylistResult result = playlist_load_into_queue(uri, *playlist, start_index, end_index, - dest, pc, secure); + dest, pc, loader); delete playlist; if (is != nullptr) diff --git a/src/playlist/PlaylistQueue.hxx b/src/playlist/PlaylistQueue.hxx index 075191ced..48e42c70e 100644 --- a/src/playlist/PlaylistQueue.hxx +++ b/src/playlist/PlaylistQueue.hxx @@ -26,6 +26,7 @@ #include "PlaylistError.hxx" +class SongLoader; class SongEnumerator; struct playlist; struct PlayerControl; @@ -43,7 +44,7 @@ PlaylistResult playlist_load_into_queue(const char *uri, SongEnumerator &e, unsigned start_index, unsigned end_index, playlist &dest, PlayerControl &pc, - bool secure); + const SongLoader &loader); /** * Opens a playlist with a playlist plugin and append to the specified @@ -53,7 +54,7 @@ PlaylistResult playlist_open_into_queue(const char *uri, unsigned start_index, unsigned end_index, playlist &dest, PlayerControl &pc, - bool secure); + const SongLoader &loader); #endif diff --git a/src/playlist/PlaylistSong.cxx b/src/playlist/PlaylistSong.cxx index b382994a6..e3defbddb 100644 --- a/src/playlist/PlaylistSong.cxx +++ b/src/playlist/PlaylistSong.cxx @@ -20,8 +20,7 @@ #include "config.h" #include "PlaylistSong.hxx" #include "Mapper.hxx" -#include "db/DatabaseSong.hxx" -#include "ls.hxx" +#include "SongLoader.hxx" #include "tag/Tag.hxx" #include "tag/TagBuilder.hxx" #include "fs/AllocatedPath.hxx" @@ -65,44 +64,22 @@ apply_song_metadata(DetachedSong &dest, const DetachedSong &src) } static bool -playlist_check_load_song(DetachedSong &song) +playlist_check_load_song(DetachedSong &song, const SongLoader &loader) { - const char *const uri = song.GetURI(); - - if (uri_has_scheme(uri)) { - return true; - } else if (PathTraitsUTF8::IsAbsolute(uri)) { - DetachedSong tmp(uri); - if (!tmp.Update()) - return false; - - apply_song_metadata(song, tmp); - return true; - } else { -#ifdef ENABLE_DATABASE - DetachedSong *tmp = DatabaseDetachSong(uri, IgnoreError()); - if (tmp == nullptr) - return false; - - apply_song_metadata(song, *tmp); - delete tmp; - return true; -#else + DetachedSong *tmp = loader.LoadSong(song.GetURI(), IgnoreError()); + if (tmp == nullptr) return false; -#endif - } + + song.SetURI(tmp->GetURI()); + apply_song_metadata(song, *tmp); + delete tmp; + return true; } bool playlist_check_translate_song(DetachedSong &song, const char *base_uri, - bool secure) + const SongLoader &loader) { - const char *const uri = song.GetURI(); - - if (uri_has_scheme(uri)) - /* valid remote song? */ - return uri_supported_scheme(uri); - if (base_uri != nullptr && strcmp(base_uri, ".") == 0) /* PathTraitsUTF8::GetParent() returns "." when there is no directory name in the given path; clear that @@ -110,26 +87,10 @@ playlist_check_translate_song(DetachedSong &song, const char *base_uri, functions */ base_uri = nullptr; - if (PathTraitsUTF8::IsAbsolute(uri)) { - /* XXX fs_charset vs utf8? */ - const char *suffix = map_to_relative_path(uri); - assert(suffix != nullptr); - - if (suffix != uri) - song.SetURI(std::string(suffix)); - else if (!secure) - /* local files must be relative to the music - directory when "secure" is enabled */ - return false; - - base_uri = nullptr; - } - - if (base_uri != nullptr) { + const char *uri = song.GetURI(); + if (base_uri != nullptr && !uri_has_scheme(uri) && + !PathTraitsUTF8::IsAbsolute(uri)) song.SetURI(PathTraitsUTF8::Build(base_uri, uri)); - /* repeat the above checks */ - return playlist_check_translate_song(song, nullptr, secure); - } - return playlist_check_load_song(song); + return playlist_check_load_song(song, loader); } diff --git a/src/playlist/PlaylistSong.hxx b/src/playlist/PlaylistSong.hxx index 2a47b28db..278df46a8 100644 --- a/src/playlist/PlaylistSong.hxx +++ b/src/playlist/PlaylistSong.hxx @@ -20,18 +20,17 @@ #ifndef MPD_PLAYLIST_SONG_HXX #define MPD_PLAYLIST_SONG_HXX +class SongLoader; class DetachedSong; /** * Verifies the song, returns false if it is unsafe. Translate the * song to a song within the database, if it is a local file. * - * @param secure if true, then local files are only allowed if they - * are relative to base_uri * @return true on success, false if the song should not be used */ bool playlist_check_translate_song(DetachedSong &song, const char *base_uri, - bool secure); + const SongLoader &loader); #endif diff --git a/src/playlist/Print.cxx b/src/playlist/Print.cxx index dc5aa252c..b5398e67d 100644 --- a/src/playlist/Print.cxx +++ b/src/playlist/Print.cxx @@ -25,6 +25,7 @@ #include "SongPrint.hxx" #include "input/InputStream.hxx" #include "DetachedSong.hxx" +#include "SongLoader.hxx" #include "fs/Traits.hxx" #include "thread/Cond.hxx" @@ -36,10 +37,12 @@ playlist_provider_print(Client &client, const char *uri, ? PathTraitsUTF8::GetParent(uri) : std::string("."); + const SongLoader loader(client); + DetachedSong *song; while ((song = e.NextSong()) != nullptr) { if (playlist_check_translate_song(*song, base_uri.c_str(), - false)) { + loader)) { if (detail) song_print_info(client, *song); else |