Commit ca36ac2b authored by Max Kellermann's avatar Max Kellermann

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.
parent ba675d6a
...@@ -168,6 +168,7 @@ src_mpd_SOURCES = \ ...@@ -168,6 +168,7 @@ src_mpd_SOURCES = \
src/ReplayGainInfo.cxx src/ReplayGainInfo.hxx \ src/ReplayGainInfo.cxx src/ReplayGainInfo.hxx \
src/DetachedSong.cxx src/DetachedSong.hxx \ src/DetachedSong.cxx src/DetachedSong.hxx \
src/SongUpdate.cxx \ src/SongUpdate.cxx \
src/SongLoader.cxx src/SongLoader.hxx \
src/SongPrint.cxx src/SongPrint.hxx \ src/SongPrint.cxx src/SongPrint.hxx \
src/SongSave.cxx src/SongSave.hxx \ src/SongSave.cxx src/SongSave.hxx \
src/StateFile.cxx src/StateFile.hxx \ src/StateFile.cxx src/StateFile.hxx \
...@@ -1732,7 +1733,9 @@ if ENABLE_DATABASE ...@@ -1732,7 +1733,9 @@ if ENABLE_DATABASE
test_test_translate_song_SOURCES = \ test_test_translate_song_SOURCES = \
src/playlist/PlaylistSong.cxx \ src/playlist/PlaylistSong.cxx \
src/PlaylistError.cxx \
src/DetachedSong.cxx \ src/DetachedSong.cxx \
src/SongLoader.cxx \
src/Log.cxx \ src/Log.cxx \
test/test_translate_song.cxx test/test_translate_song.cxx
test_test_translate_song_CPPFLAGS = $(AM_CPPFLAGS) $(CPPUNIT_CFLAGS) -DCPPUNIT_HAVE_RTTI=0 test_test_translate_song_CPPFLAGS = $(AM_CPPFLAGS) $(CPPUNIT_CFLAGS) -DCPPUNIT_HAVE_RTTI=0
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
struct Instance; struct Instance;
class MultipleOutputs; class MultipleOutputs;
class SongLoader;
/** /**
* A partition of the Music Player Daemon. It is a separate unit with * A partition of the Music Player Daemon. It is a separate unit with
...@@ -51,14 +52,10 @@ struct Partition { ...@@ -51,14 +52,10 @@ struct Partition {
playlist.Clear(pc); playlist.Clear(pc);
} }
PlaylistResult AppendFile(const char *path_utf8, PlaylistResult AppendURI(const SongLoader &loader,
unsigned *added_id=nullptr) { const char *uri_utf8,
return playlist.AppendFile(pc, path_utf8, added_id);
}
PlaylistResult AppendURI(const char *uri_utf8,
unsigned *added_id=nullptr) { 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) { PlaylistResult DeletePosition(unsigned position) {
......
...@@ -28,6 +28,7 @@ struct PlayerControl; ...@@ -28,6 +28,7 @@ struct PlayerControl;
class DetachedSong; class DetachedSong;
class Database; class Database;
class Error; class Error;
class SongLoader;
struct playlist { struct playlist {
/** /**
...@@ -149,17 +150,8 @@ public: ...@@ -149,17 +150,8 @@ public:
DetachedSong &&song, DetachedSong &&song,
unsigned *added_id=nullptr); 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, PlaylistResult AppendURI(PlayerControl &pc,
const SongLoader &loader,
const char *uri_utf8, const char *uri_utf8,
unsigned *added_id=nullptr); unsigned *added_id=nullptr);
......
...@@ -30,9 +30,8 @@ ...@@ -30,9 +30,8 @@
#include "util/UriUtil.hxx" #include "util/UriUtil.hxx"
#include "util/Error.hxx" #include "util/Error.hxx"
#include "DetachedSong.hxx" #include "DetachedSong.hxx"
#include "Mapper.hxx" #include "SongLoader.hxx"
#include "Idle.hxx" #include "Idle.hxx"
#include "db/DatabaseSong.hxx"
#include "Log.hxx" #include "Log.hxx"
#include <stdlib.h> #include <stdlib.h>
...@@ -57,17 +56,6 @@ playlist::Clear(PlayerControl &pc) ...@@ -57,17 +56,6 @@ playlist::Clear(PlayerControl &pc)
} }
PlaylistResult 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, playlist::AppendSong(PlayerControl &pc,
DetachedSong &&song, unsigned *added_id) DetachedSong &&song, unsigned *added_id)
{ {
...@@ -81,7 +69,7 @@ playlist::AppendSong(PlayerControl &pc, ...@@ -81,7 +69,7 @@ playlist::AppendSong(PlayerControl &pc,
id = queue.Append(std::move(song), 0); id = queue.Append(std::move(song), 0);
if (queue.random) { if (queue.random) {
/* shuffle the new song into the list of remaining /* shuffle the new song into the list of remaning
songs to play */ songs to play */
unsigned start; unsigned start;
...@@ -104,19 +92,19 @@ playlist::AppendSong(PlayerControl &pc, ...@@ -104,19 +92,19 @@ playlist::AppendSong(PlayerControl &pc,
PlaylistResult PlaylistResult
playlist::AppendURI(PlayerControl &pc, playlist::AppendURI(PlayerControl &pc,
const SongLoader &loader,
const char *uri, unsigned *added_id) const char *uri, unsigned *added_id)
{ {
FormatDebug(playlist_domain, "add to playlist: %s", uri); FormatDebug(playlist_domain, "add to playlist: %s", uri);
DetachedSong *song; Error error;
if (uri_has_scheme(uri)) { DetachedSong *song = loader.LoadSong(uri, error);
song = new DetachedSong(uri); if (song == nullptr) {
} else { // TODO: return the Error
#ifdef ENABLE_DATABASE LogError(error);
song = DatabaseDetachSong(uri, IgnoreError()); return error.IsDomain(playlist_domain)
if (song == nullptr) ? PlaylistResult(error.GetCode())
#endif : PlaylistResult::NO_SUCH_SONG;
return PlaylistResult::NO_SUCH_SONG;
} }
PlaylistResult result = AppendSong(pc, std::move(*song), added_id); PlaylistResult result = AppendSong(pc, std::move(*song), added_id);
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "PlaylistError.hxx" #include "PlaylistError.hxx"
#include "Playlist.hxx" #include "Playlist.hxx"
#include "DetachedSong.hxx" #include "DetachedSong.hxx"
#include "SongLoader.hxx"
#include "Mapper.hxx" #include "Mapper.hxx"
#include "Idle.hxx" #include "Idle.hxx"
#include "fs/AllocatedPath.hxx" #include "fs/AllocatedPath.hxx"
...@@ -117,19 +118,12 @@ playlist_load_spl(struct playlist &playlist, PlayerControl &pc, ...@@ -117,19 +118,12 @@ playlist_load_spl(struct playlist &playlist, PlayerControl &pc,
if (end_index > contents.size()) if (end_index > contents.size())
end_index = contents.size(); end_index = contents.size();
const SongLoader loader(nullptr);
for (unsigned i = start_index; i < end_index; ++i) { for (unsigned i = start_index; i < end_index; ++i) {
const auto &uri_utf8 = contents[i]; const auto &uri_utf8 = contents[i];
if (memcmp(uri_utf8.c_str(), "file:///", 8) == 0) { if ((playlist.AppendURI(pc, loader, uri_utf8.c_str())) != PlaylistResult::SUCCESS)
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)
FormatError(playlist_domain, FormatError(playlist_domain,
"can't add file \"%s\"", uri_utf8.c_str()); "can't add file \"%s\"", uri_utf8.c_str());
} }
......
/*
* 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
}
}
/*
* 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
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "PlaylistSave.hxx" #include "PlaylistSave.hxx"
#include "PlaylistFile.hxx" #include "PlaylistFile.hxx"
#include "db/PlaylistVector.hxx" #include "db/PlaylistVector.hxx"
#include "SongLoader.hxx"
#include "playlist/PlaylistQueue.hxx" #include "playlist/PlaylistQueue.hxx"
#include "playlist/Print.hxx" #include "playlist/Print.hxx"
#include "TimePrint.hxx" #include "TimePrint.hxx"
...@@ -65,11 +66,12 @@ handle_load(Client &client, int argc, char *argv[]) ...@@ -65,11 +66,12 @@ handle_load(Client &client, int argc, char *argv[])
} else if (!check_range(client, &start_index, &end_index, argv[2])) } else if (!check_range(client, &start_index, &end_index, argv[2]))
return CommandResult::ERROR; return CommandResult::ERROR;
const SongLoader loader(client);
const PlaylistResult result = const PlaylistResult result =
playlist_open_into_queue(argv[1], playlist_open_into_queue(argv[1],
start_index, end_index, start_index, end_index,
client.playlist, client.playlist,
client.player_control, true); client.player_control, loader);
if (result != PlaylistResult::NO_SUCH_LIST) if (result != PlaylistResult::NO_SUCH_LIST)
return print_playlist_result(client, result); return print_playlist_result(client, result);
......
...@@ -21,8 +21,9 @@ ...@@ -21,8 +21,9 @@
#include "QueueCommands.hxx" #include "QueueCommands.hxx"
#include "CommandError.hxx" #include "CommandError.hxx"
#include "db/DatabaseQueue.hxx" #include "db/DatabaseQueue.hxx"
#include "SongFilter.hxx"
#include "db/Selection.hxx" #include "db/Selection.hxx"
#include "SongFilter.hxx"
#include "SongLoader.hxx"
#include "Playlist.hxx" #include "Playlist.hxx"
#include "PlaylistPrint.hxx" #include "PlaylistPrint.hxx"
#include "client/Client.hxx" #include "client/Client.hxx"
...@@ -38,38 +39,32 @@ ...@@ -38,38 +39,32 @@
#include <string.h> #include <string.h>
CommandResult static const char *
handle_add(Client &client, gcc_unused int argc, char *argv[]) translate_uri(Client &client, const char *uri)
{ {
char *uri = argv[1]; if (memcmp(uri, "file:///", 8) == 0)
PlaylistResult result; /* drop the "file://", leave only an absolute path
(starting with a slash) */
if (memcmp(uri, "file:///", 8) == 0) { return uri + 7;
const char *path_utf8 = uri + 7;
const auto path_fs = AllocatedPath::FromUTF8(path_utf8); if (PathTraitsUTF8::IsAbsolute(uri)) {
command_error(client, ACK_ERROR_NO_EXIST, "Malformed URI");
if (path_fs.IsNull()) { return nullptr;
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 (uri_has_scheme(uri)) { return uri;
if (!uri_supported_scheme(uri)) { }
command_error(client, ACK_ERROR_NO_EXIST,
"unsupported URI scheme");
return CommandResult::ERROR;
}
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); return print_playlist_result(client, result);
} }
...@@ -88,34 +83,14 @@ handle_add(Client &client, gcc_unused int argc, char *argv[]) ...@@ -88,34 +83,14 @@ handle_add(Client &client, gcc_unused int argc, char *argv[])
CommandResult CommandResult
handle_addid(Client &client, int argc, char *argv[]) handle_addid(Client &client, int argc, char *argv[])
{ {
char *uri = argv[1]; const char *const uri = translate_uri(client, argv[1]);
unsigned added_id; if (uri == nullptr)
PlaylistResult result; return CommandResult::ERROR;
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; const SongLoader loader(client);
if (!client.AllowFile(path_fs, error))
return print_error(client, error);
result = client.partition.AppendFile(path_utf8, &added_id); unsigned added_id;
} else { auto result = client.partition.AppendURI(loader, uri, &added_id);
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);
}
if (result != PlaylistResult::SUCCESS) if (result != PlaylistResult::SUCCESS)
return print_playlist_result(client, result); return print_playlist_result(client, result);
......
...@@ -32,7 +32,7 @@ PlaylistResult ...@@ -32,7 +32,7 @@ PlaylistResult
playlist_load_into_queue(const char *uri, SongEnumerator &e, playlist_load_into_queue(const char *uri, SongEnumerator &e,
unsigned start_index, unsigned end_index, unsigned start_index, unsigned end_index,
playlist &dest, PlayerControl &pc, playlist &dest, PlayerControl &pc,
bool secure) const SongLoader &loader)
{ {
const std::string base_uri = uri != nullptr const std::string base_uri = uri != nullptr
? PathTraitsUTF8::GetParent(uri) ? PathTraitsUTF8::GetParent(uri)
...@@ -49,7 +49,7 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e, ...@@ -49,7 +49,7 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e,
} }
if (!playlist_check_translate_song(*song, base_uri.c_str(), if (!playlist_check_translate_song(*song, base_uri.c_str(),
secure)) { loader)) {
delete song; delete song;
continue; continue;
} }
...@@ -67,7 +67,7 @@ PlaylistResult ...@@ -67,7 +67,7 @@ PlaylistResult
playlist_open_into_queue(const char *uri, playlist_open_into_queue(const char *uri,
unsigned start_index, unsigned end_index, unsigned start_index, unsigned end_index,
playlist &dest, PlayerControl &pc, playlist &dest, PlayerControl &pc,
bool secure) const SongLoader &loader)
{ {
Mutex mutex; Mutex mutex;
Cond cond; Cond cond;
...@@ -80,7 +80,7 @@ playlist_open_into_queue(const char *uri, ...@@ -80,7 +80,7 @@ playlist_open_into_queue(const char *uri,
PlaylistResult result = PlaylistResult result =
playlist_load_into_queue(uri, *playlist, playlist_load_into_queue(uri, *playlist,
start_index, end_index, start_index, end_index,
dest, pc, secure); dest, pc, loader);
delete playlist; delete playlist;
if (is != nullptr) if (is != nullptr)
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "PlaylistError.hxx" #include "PlaylistError.hxx"
class SongLoader;
class SongEnumerator; class SongEnumerator;
struct playlist; struct playlist;
struct PlayerControl; struct PlayerControl;
...@@ -43,7 +44,7 @@ PlaylistResult ...@@ -43,7 +44,7 @@ PlaylistResult
playlist_load_into_queue(const char *uri, SongEnumerator &e, playlist_load_into_queue(const char *uri, SongEnumerator &e,
unsigned start_index, unsigned end_index, unsigned start_index, unsigned end_index,
playlist &dest, PlayerControl &pc, playlist &dest, PlayerControl &pc,
bool secure); const SongLoader &loader);
/** /**
* Opens a playlist with a playlist plugin and append to the specified * Opens a playlist with a playlist plugin and append to the specified
...@@ -53,7 +54,7 @@ PlaylistResult ...@@ -53,7 +54,7 @@ PlaylistResult
playlist_open_into_queue(const char *uri, playlist_open_into_queue(const char *uri,
unsigned start_index, unsigned end_index, unsigned start_index, unsigned end_index,
playlist &dest, PlayerControl &pc, playlist &dest, PlayerControl &pc,
bool secure); const SongLoader &loader);
#endif #endif
...@@ -20,8 +20,7 @@ ...@@ -20,8 +20,7 @@
#include "config.h" #include "config.h"
#include "PlaylistSong.hxx" #include "PlaylistSong.hxx"
#include "Mapper.hxx" #include "Mapper.hxx"
#include "db/DatabaseSong.hxx" #include "SongLoader.hxx"
#include "ls.hxx"
#include "tag/Tag.hxx" #include "tag/Tag.hxx"
#include "tag/TagBuilder.hxx" #include "tag/TagBuilder.hxx"
#include "fs/AllocatedPath.hxx" #include "fs/AllocatedPath.hxx"
...@@ -65,44 +64,22 @@ apply_song_metadata(DetachedSong &dest, const DetachedSong &src) ...@@ -65,44 +64,22 @@ apply_song_metadata(DetachedSong &dest, const DetachedSong &src)
} }
static bool static bool
playlist_check_load_song(DetachedSong &song) playlist_check_load_song(DetachedSong &song, const SongLoader &loader)
{ {
const char *const uri = song.GetURI(); DetachedSong *tmp = loader.LoadSong(song.GetURI(), IgnoreError());
if (tmp == nullptr)
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
return false; return false;
#endif
} song.SetURI(tmp->GetURI());
apply_song_metadata(song, *tmp);
delete tmp;
return true;
} }
bool bool
playlist_check_translate_song(DetachedSong &song, const char *base_uri, 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) if (base_uri != nullptr && strcmp(base_uri, ".") == 0)
/* PathTraitsUTF8::GetParent() returns "." when there /* PathTraitsUTF8::GetParent() returns "." when there
is no directory name in the given path; clear that is no directory name in the given path; clear that
...@@ -110,26 +87,10 @@ playlist_check_translate_song(DetachedSong &song, const char *base_uri, ...@@ -110,26 +87,10 @@ playlist_check_translate_song(DetachedSong &song, const char *base_uri,
functions */ functions */
base_uri = nullptr; base_uri = nullptr;
if (PathTraitsUTF8::IsAbsolute(uri)) { const char *uri = song.GetURI();
/* XXX fs_charset vs utf8? */ if (base_uri != nullptr && !uri_has_scheme(uri) &&
const char *suffix = map_to_relative_path(uri); !PathTraitsUTF8::IsAbsolute(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) {
song.SetURI(PathTraitsUTF8::Build(base_uri, 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);
} }
...@@ -20,18 +20,17 @@ ...@@ -20,18 +20,17 @@
#ifndef MPD_PLAYLIST_SONG_HXX #ifndef MPD_PLAYLIST_SONG_HXX
#define MPD_PLAYLIST_SONG_HXX #define MPD_PLAYLIST_SONG_HXX
class SongLoader;
class DetachedSong; class DetachedSong;
/** /**
* Verifies the song, returns false if it is unsafe. Translate the * Verifies the song, returns false if it is unsafe. Translate the
* song to a song within the database, if it is a local file. * 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 * @return true on success, false if the song should not be used
*/ */
bool bool
playlist_check_translate_song(DetachedSong &song, const char *base_uri, playlist_check_translate_song(DetachedSong &song, const char *base_uri,
bool secure); const SongLoader &loader);
#endif #endif
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "SongPrint.hxx" #include "SongPrint.hxx"
#include "input/InputStream.hxx" #include "input/InputStream.hxx"
#include "DetachedSong.hxx" #include "DetachedSong.hxx"
#include "SongLoader.hxx"
#include "fs/Traits.hxx" #include "fs/Traits.hxx"
#include "thread/Cond.hxx" #include "thread/Cond.hxx"
...@@ -36,10 +37,12 @@ playlist_provider_print(Client &client, const char *uri, ...@@ -36,10 +37,12 @@ playlist_provider_print(Client &client, const char *uri,
? PathTraitsUTF8::GetParent(uri) ? PathTraitsUTF8::GetParent(uri)
: std::string("."); : std::string(".");
const SongLoader loader(client);
DetachedSong *song; DetachedSong *song;
while ((song = e.NextSong()) != nullptr) { while ((song = e.NextSong()) != nullptr) {
if (playlist_check_translate_song(*song, base_uri.c_str(), if (playlist_check_translate_song(*song, base_uri.c_str(),
false)) { loader)) {
if (detail) if (detail)
song_print_info(client, *song); song_print_info(client, *song);
else else
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#include "config.h" #include "config.h"
#include "playlist/PlaylistSong.hxx" #include "playlist/PlaylistSong.hxx"
#include "DetachedSong.hxx" #include "DetachedSong.hxx"
#include "SongLoader.hxx"
#include "client/Client.hxx"
#include "tag/TagBuilder.hxx" #include "tag/TagBuilder.hxx"
#include "tag/Tag.hxx" #include "tag/Tag.hxx"
#include "util/Domain.hxx" #include "util/Domain.hxx"
...@@ -12,6 +14,7 @@ ...@@ -12,6 +14,7 @@
#include "ls.hxx" #include "ls.hxx"
#include "Log.hxx" #include "Log.hxx"
#include "db/DatabaseSong.hxx" #include "db/DatabaseSong.hxx"
#include "util/Error.hxx"
#include "Mapper.hxx" #include "Mapper.hxx"
#include <cppunit/TestFixture.h> #include <cppunit/TestFixture.h>
...@@ -133,6 +136,15 @@ DetachedSong::Update() ...@@ -133,6 +136,15 @@ DetachedSong::Update()
return false; return false;
} }
bool
Client::AllowFile(gcc_unused Path path_fs, gcc_unused Error &error) const
{
/* always return false, so a SongLoader with a non-nullptr
Client pointer will be regarded "insecure", while one with
client==nullptr will allow all files */
return false;
}
static std::string static std::string
ToString(const Tag &tag) ToString(const Tag &tag)
{ {
...@@ -198,65 +210,84 @@ class TranslateSongTest : public CppUnit::TestFixture { ...@@ -198,65 +210,84 @@ class TranslateSongTest : public CppUnit::TestFixture {
void TestAbsoluteURI() { void TestAbsoluteURI() {
DetachedSong song1("http://example.com/foo.ogg"); DetachedSong song1("http://example.com/foo.ogg");
auto se = ToString(song1); auto se = ToString(song1);
CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored", false)); const SongLoader loader(nullptr);
CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored",
loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); CPPUNIT_ASSERT_EQUAL(se, ToString(song1));
} }
void TestInsecure() { void TestInsecure() {
/* illegal because secure=false */ /* illegal because secure=false */
DetachedSong song1 (uri1); DetachedSong song1 (uri1);
CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr, false)); const SongLoader loader(reinterpret_cast<const Client *>(1));
CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr,
loader));
} }
void TestSecure() { void TestSecure() {
DetachedSong song1(uri1, MakeTag1b()); DetachedSong song1(uri1, MakeTag1b());
auto s1 = ToString(song1); auto s1 = ToString(song1);
auto se = ToString(DetachedSong(uri1, MakeTag1c())); auto se = ToString(DetachedSong(uri1, MakeTag1c()));
CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored", true));
const SongLoader loader(nullptr);
CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored",
loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); CPPUNIT_ASSERT_EQUAL(se, ToString(song1));
} }
void TestInDatabase() { void TestInDatabase() {
const SongLoader loader(nullptr);
DetachedSong song1("doesntexist"); DetachedSong song1("doesntexist");
CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr, false)); CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr,
loader));
DetachedSong song2(uri2, MakeTag2b()); DetachedSong song2(uri2, MakeTag2b());
auto s1 = ToString(song2); auto s1 = ToString(song2);
auto se = ToString(DetachedSong(uri2, MakeTag2c())); auto se = ToString(DetachedSong(uri2, MakeTag2c()));
CPPUNIT_ASSERT(playlist_check_translate_song(song2, nullptr, false)); CPPUNIT_ASSERT(playlist_check_translate_song(song2, nullptr,
loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); CPPUNIT_ASSERT_EQUAL(se, ToString(song2));
DetachedSong song3("/music/foo/bar.ogg", MakeTag2b()); DetachedSong song3("/music/foo/bar.ogg", MakeTag2b());
s1 = ToString(song3); s1 = ToString(song3);
se = ToString(DetachedSong(uri2, MakeTag2c())); se = ToString(DetachedSong(uri2, MakeTag2c()));
CPPUNIT_ASSERT(playlist_check_translate_song(song3, nullptr, false)); CPPUNIT_ASSERT(playlist_check_translate_song(song3, nullptr,
loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song3)); CPPUNIT_ASSERT_EQUAL(se, ToString(song3));
} }
void TestRelative() { void TestRelative() {
const SongLoader secure_loader(nullptr);
const SongLoader insecure_loader(reinterpret_cast<const Client *>(1));
/* map to music_directory */ /* map to music_directory */
DetachedSong song1("bar.ogg", MakeTag2b()); DetachedSong song1("bar.ogg", MakeTag2b());
auto s1 = ToString(song1); auto s1 = ToString(song1);
auto se = ToString(DetachedSong(uri2, MakeTag2c())); auto se = ToString(DetachedSong(uri2, MakeTag2c()));
CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/music/foo", false)); CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/music/foo",
insecure_loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); CPPUNIT_ASSERT_EQUAL(se, ToString(song1));
/* illegal because secure=false */ /* illegal because secure=false */
DetachedSong song2("bar.ogg", MakeTag2b()); DetachedSong song2("bar.ogg", MakeTag2b());
CPPUNIT_ASSERT(!playlist_check_translate_song(song1, "/foo", false)); CPPUNIT_ASSERT(!playlist_check_translate_song(song1, "/foo",
insecure_loader));
/* legal because secure=true */ /* legal because secure=true */
DetachedSong song3("bar.ogg", MakeTag1b()); DetachedSong song3("bar.ogg", MakeTag1b());
s1 = ToString(song3); s1 = ToString(song3);
se = ToString(DetachedSong(uri1, MakeTag1c())); se = ToString(DetachedSong(uri1, MakeTag1c()));
CPPUNIT_ASSERT(playlist_check_translate_song(song3, "/foo", true)); CPPUNIT_ASSERT(playlist_check_translate_song(song3, "/foo",
secure_loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song3)); CPPUNIT_ASSERT_EQUAL(se, ToString(song3));
/* relative to http:// */ /* relative to http:// */
DetachedSong song4("bar.ogg", MakeTag2a()); DetachedSong song4("bar.ogg", MakeTag2a());
s1 = ToString(song4); s1 = ToString(song4);
se = ToString(DetachedSong("http://example.com/foo/bar.ogg", MakeTag2a())); se = ToString(DetachedSong("http://example.com/foo/bar.ogg", MakeTag2a()));
CPPUNIT_ASSERT(playlist_check_translate_song(song4, "http://example.com/foo", false)); CPPUNIT_ASSERT(playlist_check_translate_song(song4, "http://example.com/foo",
insecure_loader));
CPPUNIT_ASSERT_EQUAL(se, ToString(song4)); CPPUNIT_ASSERT_EQUAL(se, ToString(song4));
} }
}; };
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment