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.

15 files changed, 279 insertions, 179 deletions

diff --git a/Makefile.am b/Makefile.am
index 1dd97a5bf..6f2e9e388 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -168,6 +168,7 @@ src_mpd_SOURCES = \
 	src/ReplayGainInfo.cxx src/ReplayGainInfo.hxx \
 	src/DetachedSong.cxx src/DetachedSong.hxx \
 	src/SongUpdate.cxx \
+	src/SongLoader.cxx src/SongLoader.hxx \
 	src/SongPrint.cxx src/SongPrint.hxx \
 	src/SongSave.cxx src/SongSave.hxx \
 	src/StateFile.cxx src/StateFile.hxx \
@@ -1732,7 +1733,9 @@ if ENABLE_DATABASE
 
 test_test_translate_song_SOURCES = \
 	src/playlist/PlaylistSong.cxx \
+	src/PlaylistError.cxx \
 	src/DetachedSong.cxx \
+	src/SongLoader.cxx \
 	src/Log.cxx \
 	test/test_translate_song.cxx
 test_test_translate_song_CPPFLAGS = $(AM_CPPFLAGS) $(CPPUNIT_CFLAGS) -DCPPUNIT_HAVE_RTTI=0
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
diff --git a/test/test_translate_song.cxx b/test/test_translate_song.cxx
index 006bd56aa..c264c0d1a 100644
--- a/test/test_translate_song.cxx
+++ b/test/test_translate_song.cxx
@@ -5,6 +5,8 @@
 #include "config.h"
 #include "playlist/PlaylistSong.hxx"
 #include "DetachedSong.hxx"
+#include "SongLoader.hxx"
+#include "client/Client.hxx"
 #include "tag/TagBuilder.hxx"
 #include "tag/Tag.hxx"
 #include "util/Domain.hxx"
@@ -12,6 +14,7 @@
 #include "ls.hxx"
 #include "Log.hxx"
 #include "db/DatabaseSong.hxx"
+#include "util/Error.hxx"
 #include "Mapper.hxx"
 
 #include <cppunit/TestFixture.h>
@@ -133,6 +136,15 @@ DetachedSong::Update()
 	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
 ToString(const Tag &tag)
 {
@@ -198,65 +210,84 @@ class TranslateSongTest : public CppUnit::TestFixture {
 	void TestAbsoluteURI() {
 		DetachedSong song1("http://example.com/foo.ogg");
 		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));
 	}
 
 	void TestInsecure() {
 		/* illegal because secure=false */
 		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() {
 		DetachedSong song1(uri1, MakeTag1b());
 		auto s1 = ToString(song1);
 		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));
 	}
 
 	void TestInDatabase() {
+		const SongLoader loader(nullptr);
+
 		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());
 		auto s1 = ToString(song2);
 		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));
 
 		DetachedSong song3("/music/foo/bar.ogg", MakeTag2b());
 		s1 = ToString(song3);
 		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));
 	}
 
 	void TestRelative() {
+		const SongLoader secure_loader(nullptr);
+		const SongLoader insecure_loader(reinterpret_cast<const Client *>(1));
+
 		/* map to music_directory */
 		DetachedSong song1("bar.ogg", MakeTag2b());
 		auto s1 = ToString(song1);
 		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));
 
 		/* illegal because secure=false */
 		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 */
 		DetachedSong song3("bar.ogg", MakeTag1b());
 		s1 = ToString(song3);
 		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));
 
 		/* relative to http:// */
 		DetachedSong song4("bar.ogg", MakeTag2a());
 		s1 = ToString(song4);
 		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));
 	}
 };