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

diff --git a/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp b/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp
index e4e97d01f6c97106110c1140e2acdd7a2b0d9c28..c31e10767c052c078260d6e339833c5feb54032a 100644 (file)
--- a/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp
+++ b/taglib/mpeg/id3v2/frames/attachedpictureframe.cpp
@@ -152,7 +152,7 @@ ByteVector AttachedPictureFrame::renderFields() const
 {
   ByteVector data;
 
-  String::Type encoding = checkEncoding(d->description, d->textEncoding);
+  String::Type encoding = checkTextEncoding(d->description, d->textEncoding);
 
   data.append(char(encoding));
   data.append(d->mimeType.data(String::Latin1));

diff --git a/taglib/mpeg/id3v2/frames/commentsframe.cpp b/taglib/mpeg/id3v2/frames/commentsframe.cpp
index 406598d99b69684ef610233d2de680b50e4ea0f9..377a7ee3c3b97b50034f85ae4d1db05c48908e7f 100644 (file)
--- a/taglib/mpeg/id3v2/frames/commentsframe.cpp
+++ b/taglib/mpeg/id3v2/frames/commentsframe.cpp
@@ -155,8 +155,8 @@ ByteVector CommentsFrame::renderFields() const
 
   String::Type encoding = d->textEncoding;
 
-  encoding = checkEncoding(d->description, encoding);
-  encoding = checkEncoding(d->text, encoding);
+  encoding = checkTextEncoding(d->description, encoding);
+  encoding = checkTextEncoding(d->text, encoding);
 
   v.append(char(encoding));
   v.append(d->language.size() == 3 ? d->language : "XXX");

diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp
index 7c2ab90955d5ffa57a9ee12ee4dd431f7aaf6753..cf72492211bbe003b34c4b1bea2ae1dd1736731b 100644 (file)
--- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp
+++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp
@@ -139,7 +139,7 @@ void TextIdentificationFrame::parseFields(const ByteVector &data)
 
 ByteVector TextIdentificationFrame::renderFields() const
 {
-  String::Type encoding = checkEncoding(d->fieldList, d->textEncoding);
+  String::Type encoding = checkTextEncoding(d->fieldList, d->textEncoding);
 
   ByteVector v;
 

diff --git a/taglib/mpeg/id3v2/frames/urllinkframe.cpp b/taglib/mpeg/id3v2/frames/urllinkframe.cpp
index 7756c4ad9e4ce6763f117f7e39f2bf3310106d27..09edec40d65fa700e205cd9ad89d9484e2d1d0eb 100644 (file)
--- a/taglib/mpeg/id3v2/frames/urllinkframe.cpp
+++ b/taglib/mpeg/id3v2/frames/urllinkframe.cpp
@@ -175,7 +175,7 @@ ByteVector UserUrlLinkFrame::renderFields() const
 {
   ByteVector v;
 
-  String::Type encoding = checkEncoding(d->description, d->textEncoding);
+  String::Type encoding = checkTextEncoding(d->description, d->textEncoding);
 
   v.append(char(encoding));
   v.append(d->description.data(encoding));

diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp
index ef8e3eed0b451de31695b412c682860d71c25135..5955917818e6d362a9ee505cf9554b03eda121da 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2frame.cpp
+++ b/taglib/mpeg/id3v2/id3v2frame.cpp
@@ -230,19 +230,38 @@ String Frame::readStringField(const ByteVector &data, String::Type encoding, int
 
 String::Type Frame::checkEncoding(const StringList &fields, String::Type encoding) // static
 {
+  return checkEncoding(fields, encoding, 4);
+}
+
+String::Type Frame::checkEncoding(const StringList &fields, String::Type encoding, uint version) // static
+{
+  if((encoding == String::UTF8 || encoding == String::UTF16BE) && version != 4)
+    return String::UTF16;
+
   if(encoding != String::Latin1)
     return encoding;
 
   for(StringList::ConstIterator it = fields.begin(); it != fields.end(); ++it) {
     if(!(*it).isLatin1()) {
-      debug("Frame::checkEncoding() -- Rendering using UTF8.");
-      return String::UTF8;
+      if(version == 4) {
+        debug("Frame::checkEncoding() -- Rendering using UTF8.");
+        return String::UTF8;
+      }
+      else {
+        debug("Frame::checkEncoding() -- Rendering using UTF16.");
+        return String::UTF16;
+      }
     }
   }
 
   return String::Latin1;
 }
 
+String::Type Frame::checkTextEncoding(const StringList &fields, String::Type encoding) const
+{
+  return checkEncoding(fields, encoding, header()->version());
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Frame::Header class
 ////////////////////////////////////////////////////////////////////////////////

diff --git a/taglib/mpeg/id3v2/id3v2frame.h b/taglib/mpeg/id3v2/id3v2frame.h
index cd401c092e55de46399321ce94fcfdbf6ece3cd2..2b6bcd88869e74c6c370ac992ec34f31b8fe3b5d 100644 (file)
--- a/taglib/mpeg/id3v2/id3v2frame.h
+++ b/taglib/mpeg/id3v2/id3v2frame.h
@@ -199,9 +199,29 @@ namespace TagLib {
        * Checks a the list of string values to see if they can be used with the
        * specified encoding and returns the recommended encoding.
        */
+      // BIC: remove and make non-static
       static String::Type checkEncoding(const StringList &fields,
                                         String::Type encoding);
 
+      /*!
+       * Checks a the list of string values to see if they can be used with the
+       * specified encoding and returns the recommended encoding. This method
+       * also checks the ID3v2 version and makes sure the encoding can be used
+       * in the specified version.
+       */
+      // BIC: remove and make non-static
+      static String::Type checkEncoding(const StringList &fields,
+                                        String::Type encoding, uint version);
+
+      /*!
+       * Checks a the list of string values to see if they can be used with the
+       * specified encoding and returns the recommended encoding. This method
+       * also checks the ID3v2 version and makes sure the encoding can be used
+       * in the version specified by the frame's header.
+       */
+      String::Type checkTextEncoding(const StringList &fields,
+                                     String::Type encoding) const;
+
     private:
       Frame(const Frame &);
       Frame &operator=(const Frame &);

diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp
index 4276963d3567160e8002b635ac049868567c2a17..3460168c6356608be2ecfc2f9e1703bd16a9fd4c 100644 (file)
--- a/tests/test_id3v2.cpp
+++ b/tests/test_id3v2.cpp
@@ -1,9 +1,12 @@
 #include <cppunit/extensions/HelperMacros.h>
 #include <string>
 #include <stdio.h>
+// so evil :(
+#define protected public
 #include <id3v2tag.h>
 #include <mpegfile.h>
 #include <id3v2frame.h>
+#undef protected
 #include <uniquefileidentifierframe.h>
 #include <textidentificationframe.h>
 #include <attachedpictureframe.h>
@@ -33,6 +36,7 @@ class TestID3v2 : public CppUnit::TestFixture
 {
   CPPUNIT_TEST_SUITE(TestID3v2);
   CPPUNIT_TEST(testUnsynchDecode);
+  CPPUNIT_TEST(testDowngradeUTF8ForID3v23);
   CPPUNIT_TEST(testUTF16BEDelimiter);
   CPPUNIT_TEST(testUTF16Delimiter);
   CPPUNIT_TEST(testReadStringField);
@@ -73,6 +77,20 @@ public:
     CPPUNIT_ASSERT_EQUAL(String("My babe just cares for me"), f.tag()->title());
   }
 
+  void testDowngradeUTF8ForID3v23()
+  {
+    ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF8);
+    StringList sl;
+    sl.append("Foo");
+    f.setText(sl);
+    f.header()->setVersion(3);
+    ByteVector data = f.render();
+    CPPUNIT_ASSERT_EQUAL((unsigned int)(4+4+2+1+6+2), data.size());
+    ID3v2::TextIdentificationFrame f2(data);
+    CPPUNIT_ASSERT_EQUAL(sl, f2.fieldList());
+    CPPUNIT_ASSERT_EQUAL(String::UTF16, f2.textEncoding());
+  }
+
   void testUTF16BEDelimiter()
   {
     ID3v2::TextIdentificationFrame f(ByteVector("TPE1"), String::UTF16BE);