command/{Queue,File}Commands: drop "file:///" prefix for absolute paths

Requiring this prefix makes the client's intention very clear, but it was too hard to understand why this prefix was needed. Initially, my intention was to differentiate from broken clients which prefix relate URIs with a slash; once MPD allowed that. In the past few years however, MPD has disallowed that, and there was no significant breakage (except for the "add /" special case which some clients apparently still do). So I figure it's about time to define that an URI that begins with a slash points to an arbitrary file on the file system.

command/{Queue,File}Commands: drop "file:///" prefix for absolute paths
676dfabc · Max Kellermann · 60e6d1d6 · 676dfabc · 676dfabc · 676dfabc
Commit 676dfabc authored Feb 09, 2015 by Max Kellermann
Hide whitespace changes
Inline Side-by-side

Showing with 18 additions and 16 deletions

NEWS NEWS +1 -0

protocol.xml doc/protocol.xml +2 -3

FileCommands.cxx src/command/FileCommands.cxx +12 -1

QueueCommands.cxx src/command/QueueCommands.cxx +3 -12

No files found.
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ ver 0.20 (not yet released)
  - "search"/"find" have a "window" parameter
  - report song duration with milliseconds precision
  - "sticker find" can match sticker values
+  - drop the "file:///" prefix for absolute file paths
 * tags
  - ape, ogg: drop support for non-standard tag "album artist"
    affected filetypes: vorbis, flac, opus & all files with ape2 tags

--- a/doc/protocol.xml
+++ b/doc/protocol.xml
@@ -1772,7 +1772,7 @@ OK
            <para>
              Clients that are connected via UNIX domain socket may
              use this command to read the tags of an arbitrary local
-              file (URI beginning with "file:///").
+              file (URI is an absolute path).
            </para>
          </listitem>
        </varlistentry>
@@ -1787,8 +1787,7 @@ OK
            <para>
              Read "comments" (i.e. key-value pairs) from the file
              specified by "URI".  This "URI" can be a path relative
-              to the music directory or a URL in the form
-              "file:///foo/bar.ogg".
+              to the music directory or an absolute path.
            </para>
            <para>
              This command may be used to list metadata of remote

--- a/src/command/FileCommands.cxx
+++ b/src/command/FileCommands.cxx
@@ -202,11 +202,22 @@ read_file_comments(Client &client, const Path path_fs)

 }

+static const char *
+translate_uri(const char *uri)
+{
+	if (memcmp(uri, "file:///", 8) == 0)
+		/* drop the "file://", leave only an absolute path
+		   (starting with a slash) */
+		return uri + 7;
+
+	return uri;
+}
+
 CommandResult
 handle_read_comments(Client &client, ConstBuffer<const char *> args)
 {
 	assert(args.size == 1);
-	const char *const uri = args.front();
+	const char *const uri = translate_uri(args.front());

 	if (memcmp(uri, "file:///", 8) == 0) {
 		/* read comments from arbitrary local file */

--- a/src/command/QueueCommands.cxx
+++ b/src/command/QueueCommands.cxx
@@ -43,18 +43,13 @@
 #include <string.h>

 static const char *
-translate_uri(Client &client, const char *uri)
+translate_uri(const char *uri)
 {
 	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;
-	}
-
 	return uri;
 }

@@ -70,9 +65,7 @@ handle_add(Client &client, ConstBuffer<const char *> args)
 		   here */
 		uri = "";

-	uri = translate_uri(client, uri);
-	if (uri == nullptr)
-		return CommandResult::ERROR;
+	uri = translate_uri(uri);

 	if (uri_has_scheme(uri) || PathTraitsUTF8::IsAbsolute(uri)) {
 		const SongLoader loader(client);
@@ -101,9 +94,7 @@ handle_add(Client &client, ConstBuffer<const char *> args)
 CommandResult
 handle_addid(Client &client, ConstBuffer<const char *> args)
 {
-	const char *const uri = translate_uri(client, args.front());
-	if (uri == nullptr)
-		return CommandResult::ERROR;
+	const char *const uri = translate_uri(args.front());

 	const SongLoader loader(client);
 	Error error;