release v0.22.2

-----BEGIN PGP SIGNATURE-----
 
 iQJEBAABCgAuFiEEA5IzWngIOJSkMBxDI26KWMbbRRIFAl+Zm30QHG1heEBtdXNp
 Y3BkLm9yZwAKCRAjbopYxttFEm0BD/0eeuI09j2LPCCQQdOUxl3ydWDLVVqDbT/b
 wUYLFEIscr0Kr8k4Mcl3C5LBEeCJq4ZYQ/VqlgNVC11SXVhq7IHJALJ3rYQVIpx+
 7MGfd1M0G39d9nlEFeKG2iNKx78aiBTqkMC9MtctacIOiKmntXP0cZZxpqGMMVZs
 5t8l8i2CM35BWisu0PjSvPJMzGVf9lgEIqx1ejHBFrX6V6DEZ8d6KEbV9FulNo1Y
 bH1UmKdsz71LC1ZE9EescmL5kVvaXyWyJmDDiu2/sqcMRjAHl2+RIAsunv6SqdGH
 Sk4OZbtXB8M8EQHuSJESUS6SnQEyGdwCptBn4NHXkajzRJqKCq0FRDyQGT+qrmUr
 H+91rZeWRYFbBMStP9l9MMQiTTjXgfCnHExDK9AmrMhiuYnuVxV1deEP7FXswCkd
 yU3sPpcKgdzMEYzoTuzXJVyMIKr0WOmNGFMEUbxfBXZDCbT1i9SH+Bi0kQAYe9h9
 JQTSWeS5NDRcI/b6nHS6ccUGFEp1scbXQNLw+17UvrUwfeZW9N9/t6jPS4kdUNEm
 tnNmbM/3o7yT3B8BGKTl5qBVSjCcgqKfsBXvy9Qn2zE0TN4HQPiJSDXZEH9LuZR1
 c6b8aI94gZkN4av1OAjmZgLyn+Pr2gyrei4FQNXyjNJkV5F3wiiLhhDTEMc2WLNm
 Zf4qpZArww==
 =eRNp
 -----END PGP SIGNATURE-----

Merge tag 'v0.22.2' into master

release v0.22.2

NEWS

@@ -2,6 +2,17 @@ ver 0.23 (not yet released)
 * protocol
   - new command "getvol"
 
+ver 0.22.2 (2020/10/28)
+* database
+  - simple: purge songs and virtual directories for unavailable plugins
+    on update
+* input
+  - qobuz/tidal: fix protocol errors due to newlines in error messages
+  - smbclient: disable by default due to libsmbclient crash bug
+* playlist
+  - soundcloud: fix protocol errors due to newlines in error messages
+* state_file: save on shutdown
+
 ver 0.22.1 (2020/10/17)
 * decoder
   - opus: apply the OpusHead output gain even if there is no EBU R128 tag

doc/conf.py

@@ -38,7 +38,7 @@ author = 'Max Kellermann'
 # built documents.
 #
 # The short X.Y version.
-version = '0.22.1'
+version = '0.22.2'
 # The full version, including alpha/beta/rc tags.
 release = version
 

doc/plugins.rst

@@ -71,6 +71,11 @@ Load music files from a SMB/CIFS server. It is used when
 :code:`music_directory` contains a ``smb://`` URI, for example
 :samp:`smb://myfileserver/Music`.
 
+Note that :file:`libsmbclient` has a serious bug which causes MPD to
+crash, and therefore this plugin is disabled by default and should not
+be used until the bug is fixed:
+https://bugzilla.samba.org/show_bug.cgi?id=11413
+
 nfs
 ---
 

doc/user.rst

@@ -86,7 +86,7 @@ For example, the following installs a fairly complete list of build dependencies
       libpulse-dev libshout3-dev \
       libsndio-dev \
       libmpdclient-dev \
-      libnfs-dev libsmbclient-dev \
+      libnfs-dev \
       libupnp-dev \
       libavahi-client-dev \
       libsqlite3-dev \

meson.build

@@ -213,7 +213,6 @@ log_dep = declare_dependency(
 sources = [
   version_cxx,
   'src/Main.cxx',
-  'src/protocol/Ack.cxx',
   'src/protocol/ArgParser.cxx',
   'src/protocol/Result.cxx',
   'src/command/CommandError.cxx',

meson_options.txt

@@ -92,7 +92,11 @@ option('cdio_paranoia', type: 'feature', description: 'libcdio_paranoia input pl
 option('curl', type: 'feature', description: 'HTTP client using CURL')
 option('mms', type: 'feature', description: 'MMS protocol support using libmms')
 option('nfs', type: 'feature', description: 'NFS protocol support using libnfs')
-option('smbclient', type: 'feature', description: 'SMB support using libsmbclient')
+
+# The "smbclient" plugin is disabled by default because libsmbclient
+# has a serious bug which crashes MPD very quickly:
+# https://bugzilla.samba.org/show_bug.cgi?id=11413
+option('smbclient', type: 'feature', value: 'disabled', description: 'SMB support using libsmbclient')
 
 #
 # Commercial services

src/Main.cxx

@@ -533,6 +533,9 @@ MainConfigured(const struct options &options, const ConfigData &raw_config)
 
 	/* cleanup */
 
+	if (instance.state_file)
+		instance.state_file->Write();
+
 	instance.BeginShutdownUpdate();
 
 	ZeroconfDeinit();

src/SongUpdate.cxx

@@ -23,6 +23,7 @@
 #include "db/plugins/simple/Directory.hxx"
 #include "storage/StorageInterface.hxx"
 #include "storage/FileInfo.hxx"
+#include "decoder/DecoderList.hxx"
 #include "fs/AllocatedPath.hxx"
 #include "fs/FileInfo.hxx"
 #include "tag/Builder.hxx"
@@ -40,6 +41,14 @@
 
 #ifdef ENABLE_DATABASE
 
+bool
+Song::IsPluginAvailable() const noexcept
+{
+	const char *suffix = GetFilenameSuffix();
+	return suffix != nullptr &&
+		decoder_plugins_supports_suffix(suffix);
+}
+
 SongPtr
 Song::LoadFile(Storage &storage, const char *path_utf8, Directory &parent)
 {

src/db/meson.build

@@ -31,6 +31,7 @@ db_glue_sources = [
   'update/Remove.cxx',
   'update/ExcludeList.cxx',
   'update/VirtualDirectory.cxx',
+  'update/SpecialDirectory.cxx',
   'DatabaseGlue.cxx',
   'Configured.cxx',
   'DatabaseSong.cxx',

src/db/plugins/simple/Directory.hxx

@@ -132,6 +132,14 @@ public:
 		return mounted_database != nullptr;
 	}
 
+	/**
+	 * Checks whether this is a "special" directory
+	 * (e.g. #DEVICE_PLAYLIST) and whether the underlying plugin
+	 * is available.
+	 */
+	gcc_pure
+	bool IsPluginAvailable() const noexcept;
+
 	/**
 	 * Remove this #Directory object from its parent and free it.  This
 	 * must not be called with the root Directory.

src/db/plugins/simple/Song.cxx

@@ -34,6 +34,14 @@ Song::Song(DetachedSong &&other, Directory &_parent) noexcept
 {
 }
 
+const char *
+Song::GetFilenameSuffix() const noexcept
+{
+	return target.empty()
+		? PathTraitsUTF8::GetFilenameSuffix(filename.c_str())
+		: PathTraitsUTF8::GetPathSuffix(target.c_str());
+}
+
 std::string
 Song::GetURI() const noexcept
 {

src/db/plugins/simple/Song.hxx

@@ -108,6 +108,16 @@ struct Song {
 
 	Song(DetachedSong &&other, Directory &_parent) noexcept;
 
+	gcc_pure
+	const char *GetFilenameSuffix() const noexcept;
+
+	/**
+	 * Checks whether the decoder plugin for this song is
+	 * available.
+	 */
+	gcc_pure
+	bool IsPluginAvailable() const noexcept;
+
 	/**
 	 * allocate a new song structure with a local file name and attempt to
 	 * load its metadata.  If all decoder plugin fail to read its meta

src/db/update/SpecialDirectory.cxx

@@ -0,0 +1,74 @@
+/*
+ * Copyright 2003-2020 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 "db/plugins/simple/Directory.hxx"
+#include "archive/ArchiveList.hxx"
+#include "decoder/DecoderList.hxx"
+#include "playlist/PlaylistRegistry.hxx"
+#include "fs/Traits.hxx"
+
+gcc_pure
+static bool
+HaveArchivePluginForFilename(const char *filename) noexcept
+{
+#ifdef ENABLE_ARCHIVE
+	const char *suffix = PathTraitsUTF8::GetFilenameSuffix(filename);
+	return suffix != nullptr &&
+		archive_plugin_from_suffix(suffix) != nullptr;
+#else
+	(void)filename;
+	return false;
+#endif
+}
+
+gcc_pure
+static bool
+HaveContainerPluginForFilename(const char *filename) noexcept
+{
+	const char *suffix = PathTraitsUTF8::GetFilenameSuffix(filename);
+	return suffix != nullptr &&
+		// TODO: check if this plugin really supports containers
+		decoder_plugins_supports_suffix(suffix);
+}
+
+gcc_pure
+static bool
+HavePlaylistPluginForFilename(const char *filename) noexcept
+{
+	const char *suffix = PathTraitsUTF8::GetFilenameSuffix(filename);
+	return suffix != nullptr && playlist_suffix_supported(suffix);
+}
+
+bool
+Directory::IsPluginAvailable() const noexcept
+{
+	switch (device) {
+	case DEVICE_INARCHIVE:
+		return HaveArchivePluginForFilename(GetName());
+
+	case DEVICE_CONTAINER:
+		return HaveContainerPluginForFilename(GetName());
+
+	case DEVICE_PLAYLIST:
+		return HavePlaylistPluginForFilename(GetName());
+
+	default:
+		return true;
+	}
+}

src/db/update/Walk.cxx

@@ -69,46 +69,58 @@ UpdateWalk::RemoveExcludedFromDirectory(Directory &directory,
 	const ScopeDatabaseLock protect;
 
 	directory.ForEachChildSafe([&](Directory &child){
-			const auto name_fs =
+		const auto name_fs =
-				AllocatedPath::FromUTF8(child.GetName());
+			AllocatedPath::FromUTF8(child.GetName());
 
-			if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
+		if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
-				editor.DeleteDirectory(&child);
+			editor.DeleteDirectory(&child);
-				modified = true;
+			modified = true;
-			}
+		}
-		});
+	});
 
 	directory.ForEachSongSafe([&](Song &song){
-			assert(&song.parent == &directory);
+		assert(&song.parent == &directory);
 
-			const auto name_fs = AllocatedPath::FromUTF8(song.filename);
+		const auto name_fs = AllocatedPath::FromUTF8(song.filename);
-			if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
+		if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
-				editor.DeleteSong(directory, &song);
+			editor.DeleteSong(directory, &song);
-				modified = true;
+			modified = true;
-			}
+		}
-		});
+	});
 }
 
 inline void
 UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept
 {
 	directory.ForEachChildSafe([&](Directory &child){
-			if (child.IsMount() || DirectoryExists(storage, child))
+		if (child.IsMount())
-				return;
+			/* mount points are always preserved */
+			return;
 
-			editor.LockDeleteDirectory(&child);
+		if (DirectoryExists(storage, child) &&
+		    child.IsPluginAvailable())
+			return;
 
-			modified = true;
+		/* the directory was deleted (or the plugin which
-		});
+		   handles this "virtual" directory is unavailable) */
+
+		editor.LockDeleteDirectory(&child);
+
+		modified = true;
+	});
 
 	directory.ForEachSongSafe([&](Song &song){
-			if (!directory_child_is_regular(storage, directory,
+		if (!directory_child_is_regular(storage, directory,
-							song.filename)) {
+						song.filename) ||
-				editor.LockDeleteSong(directory, &song);
+		    !song.IsPluginAvailable()) {
+			/* the song file was deleted (or the decoder
+			   plugin is unavailable) */
 
-				modified = true;
+			editor.LockDeleteSong(directory, &song);
-			}
+
-		});
+			modified = true;
+		}
+	});
 
 	for (auto i = directory.playlists.begin(),
 		     end = directory.playlists.end();

src/fs/Traits.hxx

@@ -88,6 +88,19 @@ struct PathTraitsFS {
 #endif
 	}
 
+	gcc_pure
+	static const_pointer GetFilenameSuffix(const_pointer filename) noexcept {
+		const_pointer dot = StringFindLast(filename, '.');
+		return dot != nullptr && dot > filename && dot[1] != 0
+			? dot + 1
+			: nullptr;
+	}
+
+	gcc_pure
+	static const_pointer GetPathSuffix(const_pointer path) noexcept {
+		return GetFilenameSuffix(GetBase(path));
+	}
+
 #ifdef _WIN32
 	gcc_pure gcc_nonnull_all
 	static constexpr bool IsDrive(const_pointer p) noexcept {
@@ -199,6 +212,19 @@ struct PathTraitsUTF8 {
 		return std::strrchr(p, SEPARATOR);
 	}
 
+	gcc_pure
+	static const_pointer GetFilenameSuffix(const_pointer filename) noexcept {
+		const_pointer dot = StringFindLast(filename, '.');
+		return dot != nullptr && dot > filename && dot[1] != 0
+			? dot + 1
+			: nullptr;
+	}
+
+	gcc_pure
+	static const_pointer GetPathSuffix(const_pointer path) noexcept {
+		return GetFilenameSuffix(GetBase(path));
+	}
+
 #ifdef _WIN32
 	gcc_pure gcc_nonnull_all
 	static constexpr bool IsDrive(const_pointer p) noexcept {

src/lib/yajl/Handle.cxx

@@ -0,0 +1,67 @@
+/*
+ * Copyright 2018-2020 Max Kellermann <max.kellermann@gmail.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE
+ * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "Handle.hxx"
+#include "util/RuntimeError.hxx"
+#include "util/ScopeExit.hxx"
+#include "util/StringStrip.hxx"
+
+#include <cstring>
+
+/**
+ * Strip whitespace at the beginning and end and replace newline
+ * characters which are illegal in the MPD protocol.
+ */
+static const char *
+StripErrorMessage(char *s) noexcept
+{
+	s = Strip(s);
+
+	while (auto newline = std::strchr(s, '\n'))
+		*newline = ';';
+
+	return s;
+}
+
+namespace Yajl {
+
+void
+Handle::ThrowError()
+{
+	unsigned char *str = yajl_get_error(handle, false,
+					    nullptr, 0);
+	AtScopeExit(this, str) {
+		yajl_free_error(handle, str);
+	};
+
+	throw FormatRuntimeError("Failed to parse JSON: %s",
+				 StripErrorMessage((char *)str));
+}
+
+} // namespace Yajl

src/lib/yajl/Handle.hxx

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018 Max Kellermann <max.kellermann@gmail.com>
+ * Copyright 2018-2020 Max Kellermann <max.kellermann@gmail.com>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -30,12 +30,8 @@
 #ifndef YAJL_HANDLE_HXX
 #define YAJL_HANDLE_HXX
 
-#include "util/RuntimeError.hxx"
-#include "util/ScopeExit.hxx"
-
 #include <yajl/yajl_parse.h>
 
-#include <stdexcept>
 #include <algorithm>
 
 namespace Yajl {
@@ -77,15 +73,12 @@ public:
 
 private:
 	void HandleStatus(yajl_status status) {
-		if (status == yajl_status_error) {
+		if (status == yajl_status_error)
-			unsigned char *str = yajl_get_error(handle, false,
+			ThrowError();
-							    nullptr, 0);
-			AtScopeExit(this, str) {
-				yajl_free_error(handle, str);
-			};
-			throw FormatRuntimeError("Failed to parse JSON: %s", str);
-		}
 	}
+
+	[[noreturn]]
+	void ThrowError();
 };
 
 } // namespace Yajl

src/lib/yajl/meson.build

@@ -5,6 +5,7 @@ endif
 
 yajl = static_library(
   'yajl',
+  'Handle.cxx',
   'ResponseParser.cxx',
   'ParseInputStream.cxx',
   include_directories: inc,

src/protocol/Ack.cxx

@@ -1,23 +0,0 @@
-/*
- * Copyright 2003-2020 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 "Ack.hxx"
-#include "util/Domain.hxx"
-
-const Domain ack_domain("ack");

src/protocol/Ack.hxx

@@ -25,8 +25,6 @@
 #include <stdexcept>
 #include <utility>
 
-class Domain;
-
 enum ack {
 	ACK_ERROR_NOT_LIST = 1,
 	ACK_ERROR_ARG = 2,
@@ -43,8 +41,6 @@ enum ack {
 	ACK_ERROR_EXIST = 56,
 };
 
-extern const Domain ack_domain;
-
 class ProtocolError : public std::runtime_error {
 	enum ack code;
 

src/util/FormatString.hxx

@@ -27,16 +27,14 @@
 template<typename T> class AllocatedString;
 
 /**
- * Format into a newly allocated string.  The caller frees the return
+ * Format into an #AllocatedString.
- * value with delete[].
  */
 gcc_nonnull_all
 AllocatedString<char>
 FormatStringV(const char *fmt, std::va_list args) noexcept;
 
 /**
- * Format into a newly allocated string.  The caller frees the return
+ * Format into an #AllocatedString.
- * value with delete[].
  */
 gcc_nonnull(1) gcc_printf(1,2)
 AllocatedString<char>

test/meson.build

@@ -264,7 +264,6 @@ if enable_database
   executable(
     'DumpDatabase',
     'DumpDatabase.cxx',
-    '../src/protocol/Ack.cxx',
     '../src/db/Registry.cxx',
     '../src/db/Selection.cxx',
     '../src/db/PlaylistVector.cxx',