diff --git a/NEWS b/NEWS index 409a0673c..2d9448207 100644 --- a/NEWS +++ b/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 diff --git a/doc/conf.py b/doc/conf.py index 642ab0970..fb9443dd3 100644 --- a/doc/conf.py +++ b/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 diff --git a/doc/plugins.rst b/doc/plugins.rst index e5056a7ab..107140942 100644 --- a/doc/plugins.rst +++ b/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 --- diff --git a/doc/user.rst b/doc/user.rst index 00ef9bdeb..db87ee7d0 100644 --- a/doc/user.rst +++ b/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 \ diff --git a/meson.build b/meson.build index 91896eb35..c1cf5b569 100644 --- a/meson.build +++ b/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', diff --git a/meson_options.txt b/meson_options.txt index a1c5ec1ed..36214692d 100644 --- a/meson_options.txt +++ b/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 diff --git a/src/Main.cxx b/src/Main.cxx index c96dac4b7..8405b6fce 100644 --- a/src/Main.cxx +++ b/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(); diff --git a/src/SongUpdate.cxx b/src/SongUpdate.cxx index c5701d9aa..2562bffb3 100644 --- a/src/SongUpdate.cxx +++ b/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) { diff --git a/src/db/meson.build b/src/db/meson.build index 2a0c8a5bd..69bf50cd6 100644 --- a/src/db/meson.build +++ b/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', diff --git a/src/db/plugins/simple/Directory.hxx b/src/db/plugins/simple/Directory.hxx index 7062e609c..1479b578a 100644 --- a/src/db/plugins/simple/Directory.hxx +++ b/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. diff --git a/src/db/plugins/simple/Song.cxx b/src/db/plugins/simple/Song.cxx index 3da13b9ea..eecf00277 100644 --- a/src/db/plugins/simple/Song.cxx +++ b/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 { diff --git a/src/db/plugins/simple/Song.hxx b/src/db/plugins/simple/Song.hxx index 04be80845..90334b031 100644 --- a/src/db/plugins/simple/Song.hxx +++ b/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 diff --git a/src/db/update/SpecialDirectory.cxx b/src/db/update/SpecialDirectory.cxx new file mode 100644 index 000000000..49c025d18 --- /dev/null +++ b/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; + } +} diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index 7526c6b6f..3e0677e91 100644 --- a/src/db/update/Walk.cxx +++ b/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(); diff --git a/src/fs/Traits.hxx b/src/fs/Traits.hxx index da899df63..32dcb4c60 100644 --- a/src/fs/Traits.hxx +++ b/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 { diff --git a/src/lib/yajl/Handle.cxx b/src/lib/yajl/Handle.cxx new file mode 100644 index 000000000..6a1741b10 --- /dev/null +++ b/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 diff --git a/src/lib/yajl/Handle.hxx b/src/lib/yajl/Handle.hxx index c2a148639..02fad8ea1 100644 --- a/src/lib/yajl/Handle.hxx +++ b/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 diff --git a/src/lib/yajl/meson.build b/src/lib/yajl/meson.build index 91dc046fb..9e483669b 100644 --- a/src/lib/yajl/meson.build +++ b/src/lib/yajl/meson.build @@ -5,6 +5,7 @@ endif yajl = static_library( 'yajl', + 'Handle.cxx', 'ResponseParser.cxx', 'ParseInputStream.cxx', include_directories: inc, diff --git a/src/protocol/Ack.cxx b/src/protocol/Ack.cxx deleted file mode 100644 index 5f4f42dd5..000000000 --- a/src/protocol/Ack.cxx +++ /dev/null @@ -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"); diff --git a/src/protocol/Ack.hxx b/src/protocol/Ack.hxx index 2b479058b..ed5e851fa 100644 --- a/src/protocol/Ack.hxx +++ b/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; diff --git a/src/util/FormatString.hxx b/src/util/FormatString.hxx index e1ecc8500..465ac7956 100644 --- a/src/util/FormatString.hxx +++ b/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> diff --git a/test/meson.build b/test/meson.build index a33b16d15..014899257 100644 --- a/test/meson.build +++ b/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',