decoder/OpusReader: return StringView

Since we now don't duplicate all items, we can easily remove the 64kB
limit from OpusReader::ReadString() and instead silently ignore and
skip all strings which are longer than 4 kB.

This fixes a tag duplication bug with Opus file containing a very long
`METADATA_BLOCK_PICTURE` tag, which occurred because the Opus plugin
returned false after parsing all tags, and then the MPD core fell back
to FFmpeg which scanned the tags again.

3 files changed, 25 insertions, 14 deletions

diff --git a/NEWS b/NEWS
index 319e506f4..6e28e9f4c 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,6 @@
 ver 0.21.10 (not yet released)
+* decoder
+  - opus: fix duplicate tags
 * output
   - httpd: reject some well-known URIs
 * fix crash bug (0.21.9 regression)
diff --git a/src/decoder/plugins/OpusReader.hxx b/src/decoder/plugins/OpusReader.hxx
index 12d6d467e..5f944e920 100644
--- a/src/decoder/plugins/OpusReader.hxx
+++ b/src/decoder/plugins/OpusReader.hxx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2018 The Music Player Daemon Project
+ * Copyright 2003-2019 The Music Player Daemon Project
  * http://www.musicpd.org
  *
  * This program is free software; you can redistribute it and/or modify
@@ -20,6 +20,8 @@
 #ifndef MPD_OPUS_READER_HXX
 #define MPD_OPUS_READER_HXX
 
+#include "util/StringView.hxx"
+
 #include <algorithm>
 
 #include <stdint.h>
@@ -81,18 +83,16 @@ public:
 		return ReadWord(length) && Skip(length);
 	}
 
-	char *ReadString() {
+	StringView ReadString() {
 		uint32_t length;
-		if (!ReadWord(length) || length >= 65536)
+		if (!ReadWord(length))
 			return nullptr;
 
 		const char *src = (const char *)Read(length);
 		if (src == nullptr)
 			return nullptr;
 
-		char *dest = new char[length + 1];
-		*std::copy_n(src, length, dest) = 0;
-		return dest;
+		return {src, length};
 	}
 };
 
diff --git a/src/decoder/plugins/OpusTags.cxx b/src/decoder/plugins/OpusTags.cxx
index f4dce2810..c0a14fa89 100644
--- a/src/decoder/plugins/OpusTags.cxx
+++ b/src/decoder/plugins/OpusTags.cxx
@@ -24,6 +24,8 @@
 #include "tag/ParseName.hxx"
 #include "ReplayGainInfo.hxx"
 
+#include <string>
+
 #include <stdint.h>
 #include <string.h>
 #include <stdlib.h>
@@ -91,18 +93,25 @@ ScanOpusTags(const void *data, size_t size,
 		return false;
 
 	while (n-- > 0) {
-		char *p = r.ReadString();
-		if (p == nullptr)
+		const auto s = r.ReadString();
+		if (s == nullptr)
 			return false;
 
-		char *eq = strchr(p, '=');
-		if (eq != nullptr && eq > p) {
-			*eq = 0;
+		if (s.size >= 4096)
+			continue;
+
+		const auto eq = s.Find('=');
+		if (eq == nullptr || eq == s.data)
+			continue;
+
+		auto name = s, value = s;
+		name.SetEnd(eq);
+		value.MoveFront(eq + 1);
 
-			ScanOneOpusTag(p, eq + 1, rgi, handler);
-		}
+		const std::string name2(name.data, name.size);
+		const std::string value2(value.data, value.size);
 
-		delete[] p;
+		ScanOneOpusTag(name2.c_str(), value2.c_str(), rgi, handler);
 	}
 
 	return true;