Make sure we don't write UTF8 or UTF16BE to ID3v2.3 tags.

taglib/mpeg/id3v2/frames/generalencapsulatedobjectframe.cpp

@@ -27,6 +27,7 @@
  ***************************************************************************/
 
 #include <tdebug.h>
+#include <tstringlist.h>
 
 #include "generalencapsulatedobjectframe.h"
 
@@ -152,15 +153,21 @@ void GeneralEncapsulatedObjectFrame::parseFields(const ByteVector &data)
 
 ByteVector GeneralEncapsulatedObjectFrame::renderFields() const
 {
+  StringList sl;
+  sl.append(d->fileName);
+  sl.append(d->description);
+
+  const String::Type encoding = checkTextEncoding(sl, d->textEncoding);
+
   ByteVector data;
 
-  data.append(char(d->textEncoding));
+  data.append(char(encoding));
   data.append(d->mimeType.data(String::Latin1));
   data.append(textDelimiter(String::Latin1));
-  data.append(d->fileName.data(d->textEncoding));
-  data.append(textDelimiter(d->textEncoding));
-  data.append(d->description.data(d->textEncoding));
-  data.append(textDelimiter(d->textEncoding));
+  data.append(d->fileName.data(encoding));
+  data.append(textDelimiter(encoding));
+  data.append(d->description.data(encoding));
+  data.append(textDelimiter(encoding));
   data.append(d->data);
 
   return data;

taglib/mpeg/id3v2/frames/ownershipframe.cpp

@@ -24,9 +24,10 @@
  ***************************************************************************/
 
 #include <tdebug.h>
+#include <tstringlist.h>
+#include <id3v2tag.h>
 
 #include "ownershipframe.h"
-#include <id3v2tag.h>
 
 using namespace TagLib;
 using namespace ID3v2;
@@ -113,24 +114,24 @@ void OwnershipFrame::setTextEncoding(String::Type encoding)
 void OwnershipFrame::parseFields(const ByteVector &data)
 {
   int pos = 0;
-  
+
   // Get the text encoding
   d->textEncoding = String::Type(data[0]);
   pos += 1;
-  
+
   // Read the price paid this is a null terminate string
   d->pricePaid = readStringField(data, String::Latin1, &pos);
-  
+
   // If we don't have at least 8 bytes left then don't parse the rest of the
   // data
   if(data.size() - pos < 8) {
     return;
   }
-  
+
   // Read the date purchased YYYYMMDD
   d->datePurchased = String(data.mid(pos, 8));
   pos += 8;
-  
+
   // Read the seller
   if(d->textEncoding == String::Latin1)
     d->seller = Tag::latin1StringHandler()->parse(data.mid(pos));
@@ -140,14 +141,19 @@ void OwnershipFrame::parseFields(const ByteVector &data)
 
 ByteVector OwnershipFrame::renderFields() const
 {
+  StringList sl;
+  sl.append(d->seller);
+
+  const String::Type encoding = checkTextEncoding(sl, d->textEncoding);
+
   ByteVector v;
-  
-  v.append(char(d->textEncoding));
+
+  v.append(char(encoding));
   v.append(d->pricePaid.data(String::Latin1));
   v.append(textDelimiter(String::Latin1));
   v.append(d->datePurchased.data(String::Latin1));
-  v.append(d->seller.data(d->textEncoding));
- 
+  v.append(d->seller.data(encoding));
+
   return v;
 }
 

taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp

@@ -169,13 +169,19 @@ void UnsynchronizedLyricsFrame::parseFields(const ByteVector &data)
 
 ByteVector UnsynchronizedLyricsFrame::renderFields() const
 {
+  StringList sl;
+  sl.append(d->description);
+  sl.append(d->text);
+
+  const String::Type encoding = checkTextEncoding(sl, d->textEncoding);
+
   ByteVector v;
 
-  v.append(char(d->textEncoding));
+  v.append(char(encoding));
   v.append(d->language.size() == 3 ? d->language : "XXX");
-  v.append(d->description.data(d->textEncoding));
-  v.append(textDelimiter(d->textEncoding));
-  v.append(d->text.data(d->textEncoding));
+  v.append(d->description.data(encoding));
+  v.append(textDelimiter(encoding));
+  v.append(d->text.data(encoding));
 
   return v;
 }

taglib/mpeg/id3v2/id3v2frame.cpp

@@ -97,10 +97,10 @@ unsigned int Frame::headerSize(unsigned int version)
 
 ByteVector Frame::textDelimiter(String::Type t)
 {
-  ByteVector d = char(0);
   if(t == String::UTF16 || t == String::UTF16BE || t == String::UTF16LE)
-    d.append(char(0));
-  return d;
+    return ByteVector(2, '\0');
+  else
+    return ByteVector(1, '\0');
 }
 
 const String Frame::instrumentPrefix("PERFORMER:");

tests/test_id3v2.cpp

@@ -70,7 +70,8 @@ class TestID3v2 : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestID3v2);
   CPPUNIT_TEST(testUnsynchDecode);
-  CPPUNIT_TEST(testDowngradeUTF8ForID3v23);
+  CPPUNIT_TEST(testDowngradeUTF8ForID3v23_1);
+  CPPUNIT_TEST(testDowngradeUTF8ForID3v23_2);
   CPPUNIT_TEST(testUTF16BEDelimiter);
   CPPUNIT_TEST(testUTF16Delimiter);
   CPPUNIT_TEST(testReadStringField);
@@ -130,7 +131,7 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("My babe just cares for me"), f.tag()->title());
   }
 
-  void testDowngradeUTF8ForID3v23()
+  void testDowngradeUTF8ForID3v23_1()
   {
     ScopedFileCopy copy("xing", ".mp3");
     string newname = copy.fileName();
@@ -154,6 +155,27 @@ public:
     CPPUNIT_ASSERT_EQUAL(String::UTF16, f2.textEncoding());
   }
 
+  void testDowngradeUTF8ForID3v23_2()
+  {
+    ScopedFileCopy copy("xing", ".mp3");
+
+    ID3v2::UnsynchronizedLyricsFrame *f
+      = new ID3v2::UnsynchronizedLyricsFrame(String::UTF8);
+    f->setText("Foo");
+
+    MPEG::File file(copy.fileName().c_str());
+    file.ID3v2Tag(true)->addFrame(f);
+    file.save(MPEG::File::ID3v2, true, 3);
+    CPPUNIT_ASSERT(file.hasID3v2Tag());
+
+    ByteVector data = f->render();
+    CPPUNIT_ASSERT_EQUAL((unsigned int)(4+4+2+1+3+2+2+6+2), data.size());
+
+    ID3v2::UnsynchronizedLyricsFrame f2(data);
+    CPPUNIT_ASSERT_EQUAL(String("Foo"), f2.text());
+    CPPUNIT_ASSERT_EQUAL(String::UTF16, f2.textEncoding());
+  }
+
   void testUTF16BEDelimiter()
   {
     ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF16BE);