release v0.22.2

The bug https://bugzilla.samba.org/show_bug.cgi?id=11413 makes MPD
crash after at most a minute of using the plugin.  Since this bug is
five years old already and it doesn't look like it will ever be fixed,
all libsmbclient code in MPD is scheduled for removal.  For now, the
plugin is disabled by default so people are less likely to hit the
crash bug.

Closes https://github.com/MusicPlayerDaemon/MPD/issues/991

NEWS

@@ -1,3 +1,14 @@
+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

@@ -1,7 +1,7 @@
 project(
   'mpd',
   ['c', 'cpp'],
-  version: '0.22.1',
+  version: '0.22.2',
   meson_version: '>= 0.49.0',
   default_options: [
     'c_std=c99',
@@ -224,7 +224,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 =
-				AllocatedPath::FromUTF8(child.GetName());
+		const auto name_fs =
+			AllocatedPath::FromUTF8(child.GetName());
 
-			if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
-				editor.DeleteDirectory(&child);
-				modified = true;
-			}
-		});
+		if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
+			editor.DeleteDirectory(&child);
+			modified = true;
+		}
+	});
 
 	directory.ForEachSongSafe([&](Song &song){
-			assert(&song.parent == &directory);
+		assert(&song.parent == &directory);
 
-			const auto name_fs = AllocatedPath::FromUTF8(song.filename);
-			if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
-				editor.DeleteSong(directory, &song);
-				modified = true;
-			}
-		});
+		const auto name_fs = AllocatedPath::FromUTF8(song.filename);
+		if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
+			editor.DeleteSong(directory, &song);
+			modified = true;
+		}
+	});
 }
 
 inline void
 UpdateWalk::PurgeDeletedFromDirectory(Directory &directory) noexcept
 {
 	directory.ForEachChildSafe([&](Directory &child){
-			if (child.IsMount() || DirectoryExists(storage, child))
-				return;
+		if (child.IsMount())
+			/* 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,
-							song.filename)) {
-				editor.LockDeleteSong(directory, &song);
+		if (!directory_child_is_regular(storage, directory,
+						song.filename) ||
+		    !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) {
-			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", str);
-		}
+		if (status == yajl_status_error)
+			ThrowError();
 	}
+
+	[[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
- * value with delete[].
+ * Format into an #AllocatedString.
  */
 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
- * value with delete[].
+ * Format into an #AllocatedString.
  */
 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',