From 432ce9b1de0f89e0f714d182980d5a562024faa5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 2 Nov 2014 11:41:40 +0100 Subject: [PATCH 01/11] configure.ac: prepare for 0.18.17 --- NEWS | 2 ++ configure.ac | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 2e8c2dcdb..bea9b2ebb 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,5 @@ +ver 0.18.17 (not yet released) + ver 0.18.16 (2014/09/26) * fix DSD breakage due to typo in configure.ac diff --git a/configure.ac b/configure.ac index 3d6b8526c..d6526e883 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ(2.60) -AC_INIT(mpd, 0.18.16, mpd-devel@musicpd.org) +AC_INIT(mpd, 0.18.17, mpd-devel@musicpd.org) VERSION_MAJOR=0 VERSION_MINOR=18 -VERSION_REVISION=0 +VERSION_REVISION=17 VERSION_EXTRA=0 AC_CONFIG_SRCDIR([src/Main.cxx]) From c37f7abb79b6c9f30a77ea605b18674acc5ffff2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 10 Oct 2014 22:06:48 +0200 Subject: [PATCH 02/11] TagString: use g_strndup() for unterminated string Fixes buffer overflow bug. --- src/tag/TagString.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tag/TagString.cxx b/src/tag/TagString.cxx index 3e8d8c1b0..9ab095249 100644 --- a/src/tag/TagString.cxx +++ b/src/tag/TagString.cxx @@ -33,7 +33,7 @@ patch_utf8(const char *src, size_t length, const gchar *end) { /* duplicate the string, and replace invalid bytes in that buffer */ - char *dest = g_strdup(src); + char *dest = g_strndup(src, length); do { dest[end - src] = '?'; From c50a0cf7bf96a3eb2d49e5416dfe88dc86a589ef Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Oct 2014 23:29:56 +0200 Subject: [PATCH 03/11] output/roar: remove unnecessary "volatile" keyword A mutex acts as a memory barrier, and thus "volatile" is not necessary. --- src/output/RoarOutputPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/RoarOutputPlugin.cxx b/src/output/RoarOutputPlugin.cxx index 895a165d1..20d69f3f9 100644 --- a/src/output/RoarOutputPlugin.cxx +++ b/src/output/RoarOutputPlugin.cxx @@ -46,7 +46,7 @@ class RoarOutput { struct roar_connection con; struct roar_audio_info info; mutable Mutex mutex; - volatile bool alive; + bool alive; public: RoarOutput() From 94c240a0264b7ce1693fc341778ac19ac3a535b9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 25 Oct 2014 00:19:01 +0200 Subject: [PATCH 04/11] configure.ac: show DSD in result --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index d6526e883..05a4f5050 100644 --- a/configure.ac +++ b/configure.ac @@ -1523,6 +1523,7 @@ results(un,[UNIX Domain Sockets]) printf '\nFile format support:\n\t' results(aac, [AAC]) results(adplug, [AdPlug]) +results(dsd, [DSD]) results(sidplay, [C64 SID]) results(ffmpeg, [FFMPEG]) results(flac, [FLAC]) From bccd4ef2f72f723b7abb1d7f6d004a70cad735aa Mon Sep 17 00:00:00 2001 From: Steven OBrien Date: Sun, 9 Feb 2014 15:47:45 +0000 Subject: [PATCH 05/11] decoder/ffmpeg: recognize MIME type audio/aacp --- NEWS | 2 ++ src/decoder/FfmpegDecoderPlugin.cxx | 1 + 2 files changed, 3 insertions(+) diff --git a/NEWS b/NEWS index bea9b2ebb..0706e447c 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.18.17 (not yet released) +* decoder + - ffmpeg: recognize MIME type audio/aacp ver 0.18.16 (2014/09/26) * fix DSD breakage due to typo in configure.ac diff --git a/src/decoder/FfmpegDecoderPlugin.cxx b/src/decoder/FfmpegDecoderPlugin.cxx index 9cd26c4fa..104129ad9 100644 --- a/src/decoder/FfmpegDecoderPlugin.cxx +++ b/src/decoder/FfmpegDecoderPlugin.cxx @@ -643,6 +643,7 @@ static const char *const ffmpeg_mime_types[] = { "audio/8svx", "audio/16sv", "audio/aac", + "audio/aacp", "audio/ac3", "audio/aiff" "audio/amr", From f6b2899dd2f2b7985da0cf3734a7276ea54e23a2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 25 Oct 2014 20:42:50 +0200 Subject: [PATCH 06/11] decoder/faad: remove workaround for ancient libfaad2 ABI bug Many years ago, FAAD had a serious ABI bug: the NeAACDecInit() prototype in its header declared the "samplerate" parameter to be "unsigned long *", but internally, the function assumed it was "uint32_t *" instead. On 32 bit machines, that was no difference, but on 64 bit, this left one portion of the return value uninitialized; and worse, on big-endian, the wrong word was filled. This bug had to be worked around in MPD (commit 9c4e97a6). A few months later, the bug was fixed in the FAAD CVS in commit 1.117 on file libfaad/decoder.c; the commit message was: "Use public headers internally to prevent duplicate declarations" The commit message was too brief at best; the problem was not duplicate declarations, but a prototype mismatch. No mention of the bug fix in the ChangeLog. The MPD project never learned about this bug fix, and so MPD would always pass a "uin32_t *" dressed up as a "unsigned long *". Nearly 6 years later, it's about time to fix this second ABI problem. Let's kill the workaround! --- NEWS | 1 + m4/faad.m4 | 31 +------------------------------ src/decoder/FaadDecoderPlugin.cxx | 12 ++---------- 3 files changed, 4 insertions(+), 40 deletions(-) diff --git a/NEWS b/NEWS index 0706e447c..f27bd8c4f 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ver 0.18.17 (not yet released) * decoder + - faad: remove workaround for ancient libfaad2 ABI bug - ffmpeg: recognize MIME type audio/aacp ver 0.18.16 (2014/09/26) diff --git a/m4/faad.m4 b/m4/faad.m4 index 5ca520e79..9dcb1ccab 100644 --- a/m4/faad.m4 +++ b/m4/faad.m4 @@ -62,36 +62,7 @@ int main() { CPPFLAGS=$oldcppflags fi -if test x$enable_aac = xyes; then - oldcflags=$CFLAGS - oldlibs=$LIBS - oldcppflags=$CPPFLAGS - CFLAGS="$CFLAGS $FAAD_CFLAGS -Werror" - LIBS="$LIBS $FAAD_LIBS" - CPPFLAGS=$CFLAGS - - AC_MSG_CHECKING(for broken libfaad headers) - AC_COMPILE_IFELSE([AC_LANG_SOURCE([ -#include -#include -#include - -int main() { - unsigned char channels; - uint32_t sample_rate; - - NeAACDecInit2(NULL, NULL, 0, &sample_rate, &channels); - return 0; -} - ])], - [AC_MSG_RESULT(correct)], - [AC_MSG_RESULT(broken); - AC_DEFINE(HAVE_FAAD_LONG, 1, [Define if faad.h uses the broken "unsigned long" pointers])]) - - CFLAGS=$oldcflags - LIBS=$oldlibs - CPPFLAGS=$oldcppflags -else +if test x$enable_aac = xno; then FAAD_LIBS="" FAAD_CFLAGS="" fi diff --git a/src/decoder/FaadDecoderPlugin.cxx b/src/decoder/FaadDecoderPlugin.cxx index b446ac5be..ae1181b4c 100644 --- a/src/decoder/FaadDecoderPlugin.cxx +++ b/src/decoder/FaadDecoderPlugin.cxx @@ -277,20 +277,12 @@ faad_decoder_init(NeAACDecHandle decoder, DecoderBuffer *buffer, } uint8_t channels; - uint32_t sample_rate; -#ifdef HAVE_FAAD_LONG - /* neaacdec.h declares all arguments as "unsigned long", but - internally expects uint32_t pointers. To avoid gcc - warnings, use this workaround. */ - unsigned long *sample_rate_p = (unsigned long *)(void *)&sample_rate; -#else - uint32_t *sample_rate_p = &sample_rate; -#endif + unsigned long sample_rate; long nbytes = NeAACDecInit(decoder, /* deconst hack, libfaad requires this */ const_cast(data), length, - sample_rate_p, &channels); + &sample_rate, &channels); if (nbytes < 0) { error.Set(faad_decoder_domain, "Not an AAC stream"); return false; From c882568ccd5271a3f2c9d97a9a718706f9e71a65 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 26 Oct 2014 08:14:16 +0100 Subject: [PATCH 07/11] playlist/m3u: recognize the file suffix ".m3u8" --- NEWS | 2 ++ src/playlist/ExtM3uPlaylistPlugin.cxx | 3 ++- src/playlist/M3uPlaylistPlugin.cxx | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index f27bd8c4f..439f39c4e 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.18.17 (not yet released) +* playlist + - m3u: recognize the file suffix ".m3u8" * decoder - faad: remove workaround for ancient libfaad2 ABI bug - ffmpeg: recognize MIME type audio/aacp diff --git a/src/playlist/ExtM3uPlaylistPlugin.cxx b/src/playlist/ExtM3uPlaylistPlugin.cxx index 8d260fec7..5ef010bda 100644 --- a/src/playlist/ExtM3uPlaylistPlugin.cxx +++ b/src/playlist/ExtM3uPlaylistPlugin.cxx @@ -135,7 +135,8 @@ ExtM3uPlaylist::NextSong() static const char *const extm3u_suffixes[] = { "m3u", - NULL + "m3u8", + nullptr }; static const char *const extm3u_mime_types[] = { diff --git a/src/playlist/M3uPlaylistPlugin.cxx b/src/playlist/M3uPlaylistPlugin.cxx index 3f99bdfdf..8b6adc2b6 100644 --- a/src/playlist/M3uPlaylistPlugin.cxx +++ b/src/playlist/M3uPlaylistPlugin.cxx @@ -61,6 +61,7 @@ M3uPlaylist::NextSong() static const char *const m3u_suffixes[] = { "m3u", + "m3u8", nullptr }; From 6ad336743d861a03df3079058fdc18eee07a3014 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 31 Oct 2014 14:59:27 +0100 Subject: [PATCH 08/11] PlaylistFile: don't allow empty playlist name --- NEWS | 1 + src/PlaylistFile.cxx | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/NEWS b/NEWS index 439f39c4e..509627858 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ver 0.18.17 (not yet released) * playlist + - don't allow empty playlist name - m3u: recognize the file suffix ".m3u8" * decoder - faad: remove workaround for ancient libfaad2 ABI bug diff --git a/src/PlaylistFile.cxx b/src/PlaylistFile.cxx index e7dae6258..e5285ad04 100644 --- a/src/PlaylistFile.cxx +++ b/src/PlaylistFile.cxx @@ -69,6 +69,10 @@ spl_global_init(void) bool spl_valid_name(const char *name_utf8) { + if (*name_utf8 == 0) + /* empty name not allowed */ + return false; + /* * Not supporting '/' was done out of laziness, and we should * really strive to support it in the future. From 674091424e715fddd8fbfe8146f351da5bf84974 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 1 Nov 2014 12:45:47 +0100 Subject: [PATCH 09/11] util/UriUtil: add uri_get_suffix() overload that ignores query string --- src/util/UriUtil.cxx | 17 +++++++++++++++++ src/util/UriUtil.hxx | 11 +++++++++++ test/test_util.cxx | 19 +++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/src/util/UriUtil.cxx b/src/util/UriUtil.cxx index 2609db2cf..1783fbca5 100644 --- a/src/util/UriUtil.cxx +++ b/src/util/UriUtil.cxx @@ -44,6 +44,23 @@ uri_get_suffix(const char *uri) return suffix; } +const char * +uri_get_suffix(const char *uri, UriSuffixBuffer &buffer) +{ + const char *suffix = uri_get_suffix(uri); + if (suffix == nullptr) + return nullptr; + + const char *q = strchr(suffix, '?'); + if (q != nullptr && size_t(q - suffix) < sizeof(buffer.data)) { + memcpy(buffer.data, suffix, q - suffix); + buffer.data[q - suffix] = 0; + suffix = buffer.data; + } + + return suffix; +} + static const char * verify_uri_segment(const char *p) { diff --git a/src/util/UriUtil.hxx b/src/util/UriUtil.hxx index 78d0a6bff..1c6bce3ff 100644 --- a/src/util/UriUtil.hxx +++ b/src/util/UriUtil.hxx @@ -35,6 +35,17 @@ gcc_pure const char * uri_get_suffix(const char *uri); +struct UriSuffixBuffer { + char data[8]; +}; + +/** + * Returns the file name suffix, ignoring the query string. + */ +gcc_pure +const char * +uri_get_suffix(const char *uri, UriSuffixBuffer &buffer); + /** * Returns true if this is a safe "local" URI: * diff --git a/test/test_util.cxx b/test/test_util.cxx index a472391a3..91e87957f 100644 --- a/test/test_util.cxx +++ b/test/test_util.cxx @@ -33,6 +33,25 @@ public: uri_get_suffix(".jpg")); CPPUNIT_ASSERT_EQUAL((const char *)nullptr, uri_get_suffix("/foo/.jpg")); + + /* the first overload does not eliminate the query + string */ + CPPUNIT_ASSERT_EQUAL(0, strcmp(uri_get_suffix("/foo/bar.jpg?query_string"), + "jpg?query_string")); + + /* ... but the second one does */ + UriSuffixBuffer buffer; + CPPUNIT_ASSERT_EQUAL(0, strcmp(uri_get_suffix("/foo/bar.jpg?query_string", + buffer), + "jpg")); + + /* repeat some of the above tests with the second overload */ + CPPUNIT_ASSERT_EQUAL((const char *)nullptr, + uri_get_suffix("/foo/bar", buffer)); + CPPUNIT_ASSERT_EQUAL((const char *)nullptr, + uri_get_suffix("/foo.jpg/bar", buffer)); + CPPUNIT_ASSERT_EQUAL(0, strcmp(uri_get_suffix("/foo/bar.jpg", buffer), + "jpg")); } void TestRemoveAuth() { From 32b5654a6e7738211e6aa18ab8089cc6328aa1fa Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 1 Nov 2014 13:20:39 +0100 Subject: [PATCH 10/11] Decoder, Playlist: ignore URI query string for plugin detection Use the new uri_get_suffix() overload that removes the query string. --- NEWS | 1 + src/DecoderThread.cxx | 3 ++- src/PlaylistRegistry.cxx | 11 ++++++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 509627858..1cebfd2db 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ ver 0.18.17 (not yet released) - don't allow empty playlist name - m3u: recognize the file suffix ".m3u8" * decoder + - ignore URI query string for plugin detection - faad: remove workaround for ancient libfaad2 ABI bug - ffmpeg: recognize MIME type audio/aacp diff --git a/src/DecoderThread.cxx b/src/DecoderThread.cxx index cf21534f0..7ee36faca 100644 --- a/src/DecoderThread.cxx +++ b/src/DecoderThread.cxx @@ -212,7 +212,8 @@ static bool decoder_run_stream_locked(Decoder &decoder, InputStream &is, const char *uri, bool &tried_r) { - const char *const suffix = uri_get_suffix(uri); + UriSuffixBuffer suffix_buffer; + const char *const suffix = uri_get_suffix(uri, suffix_buffer); using namespace std::placeholders; const auto f = std::bind(decoder_run_stream_plugin, diff --git a/src/PlaylistRegistry.cxx b/src/PlaylistRegistry.cxx index 9afbe349d..f81978322 100644 --- a/src/PlaylistRegistry.cxx +++ b/src/PlaylistRegistry.cxx @@ -164,12 +164,12 @@ static SongEnumerator * playlist_list_open_uri_suffix(const char *uri, Mutex &mutex, Cond &cond, const bool *tried) { - const char *suffix; SongEnumerator *playlist = nullptr; assert(uri != nullptr); - suffix = uri_get_suffix(uri); + UriSuffixBuffer suffix_buffer; + const char *const suffix = uri_get_suffix(uri, suffix_buffer); if (suffix == nullptr) return nullptr; @@ -273,8 +273,6 @@ playlist_list_open_stream_suffix(InputStream &is, const char *suffix) SongEnumerator * playlist_list_open_stream(InputStream &is, const char *uri) { - const char *suffix; - is.LockWaitReady(); const char *const mime = is.GetMimeType(); @@ -284,7 +282,10 @@ playlist_list_open_stream(InputStream &is, const char *uri) return playlist; } - suffix = uri != nullptr ? uri_get_suffix(uri) : nullptr; + UriSuffixBuffer suffix_buffer; + const char *suffix = uri != nullptr + ? uri_get_suffix(uri, suffix_buffer) + : nullptr; if (suffix != nullptr) { auto playlist = playlist_list_open_stream_suffix(is, suffix); if (playlist != nullptr) From ec3191f50279c432ffef7449133db1d4c433120c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 1 Nov 2014 14:09:30 +0100 Subject: [PATCH 11/11] input/curl: fix curl_easy_setopt() parameter types --- src/input/CurlInputPlugin.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/input/CurlInputPlugin.cxx b/src/input/CurlInputPlugin.cxx index b78545951..031ebfea6 100644 --- a/src/input/CurlInputPlugin.cxx +++ b/src/input/CurlInputPlugin.cxx @@ -983,10 +983,10 @@ input_curl_easy_init(struct input_curl *c, Error &error) input_curl_writefunction); curl_easy_setopt(c->easy, CURLOPT_WRITEDATA, c); curl_easy_setopt(c->easy, CURLOPT_HTTP200ALIASES, http_200_aliases); - curl_easy_setopt(c->easy, CURLOPT_FOLLOWLOCATION, 1); - curl_easy_setopt(c->easy, CURLOPT_NETRC, 1); - curl_easy_setopt(c->easy, CURLOPT_MAXREDIRS, 5); - curl_easy_setopt(c->easy, CURLOPT_FAILONERROR, true); + curl_easy_setopt(c->easy, CURLOPT_FOLLOWLOCATION, 1l); + curl_easy_setopt(c->easy, CURLOPT_NETRC, 1l); + curl_easy_setopt(c->easy, CURLOPT_MAXREDIRS, 5l); + curl_easy_setopt(c->easy, CURLOPT_FAILONERROR, 1l); curl_easy_setopt(c->easy, CURLOPT_ERRORBUFFER, c->error); curl_easy_setopt(c->easy, CURLOPT_NOPROGRESS, 1l); curl_easy_setopt(c->easy, CURLOPT_NOSIGNAL, 1l);