Handle RIFF chunk padding and ignore trailing garbage

This is based on patches by Marc Halbruegge, but those only deal with
read-only cases. The code now also correctly adds padding to RIFF
chunks, and calculates offsets in chunkData taking the padding into
account.

BUG:171957
BUG:175781

git-svn-id: svn://anonsvn.kde.org/home/kde/trunk/kdesupport/taglib@1003745 283d02a7-25f6-0310-bc7c-ecb5cbfe19da

diff --git a/taglib/riff/rifffile.cpp b/taglib/riff/rifffile.cpp
index 2aca44aa376b19965aaf6b81a659fd23670f72bc..25ada55265222f4355bc79344285c59766ec1ffa 100644 (file)
--- a/taglib/riff/rifffile.cpp
+++ b/taglib/riff/rifffile.cpp
@@ -49,6 +49,7 @@ public:
   std::vector<ByteVector> chunkNames;
   std::vector<uint> chunkOffsets;
   std::vector<uint> chunkSizes;
+  std::vector<char> chunkPadding;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -101,7 +102,7 @@ ByteVector RIFF::File::chunkData(uint i)
   long begin = 12 + 8;
 
   for(uint it = 0; it < i; it++)
-    begin += 8 + d->chunkSizes[it];
+    begin += 8 + d->chunkSizes[it] + d->chunkPadding[it];
 
   seek(begin);
 
@@ -128,12 +129,15 @@ void RIFF::File::setChunkData(const ByteVector &name, const ByteVector &data)
 
       // Now update the specific chunk
 
-      writeChunk(name, data, d->chunkOffsets[i] - 8, d->chunkSizes[i] + 8);
+      writeChunk(name, data, d->chunkOffsets[i] - 8, d->chunkSizes[i] + d->chunkPadding[i] + 8);
+
+      d->chunkSizes[i] = data.size();
+      d->chunkPadding[i] = (data.size() & 0x01) ? 1 : 0;
 
       // Now update the internal offsets
 
       for(i++; i < d->chunkNames.size(); i++)
-        d->chunkOffsets[i] += sizeDifference;
+        d->chunkOffsets[i] = d->chunkOffsets[i-1] + 8 + d->chunkSizes[i-1] + d->chunkPadding[i-1];
 
       return;
     }
@@ -158,16 +162,38 @@ void RIFF::File::read()
   d->size = readBlock(4).toUInt(bigEndian);
   d->format = readBlock(4);
 
-  while(tell() < length()) {
+  // + 8: chunk header at least, fix for additional junk bytes
+  while(tell() + 8 <= length()) {
     ByteVector chunkName = readBlock(4);
     uint chunkSize = readBlock(4).toUInt(bigEndian);
 
+    if(tell() + chunkSize > length()) {
+      // something wrong
+      break;
+    }
+
     d->chunkNames.push_back(chunkName);
     d->chunkSizes.push_back(chunkSize);
 
     d->chunkOffsets.push_back(tell());
 
     seek(chunkSize, Current);
+
+    // check padding
+    char paddingSize = 0;
+    long uPosNotPadded = tell();
+    if((uPosNotPadded & 0x01) != 0) {
+      ByteVector iByte = readBlock(1);
+      if((iByte.size() != 1) || (iByte[0] != 0)) {
+        // not well formed, re-seek
+        seek(uPosNotPadded, Beginning);
+      }
+      else {
+        paddingSize = 1;
+      }
+    }
+    d->chunkPadding.push_back(paddingSize);
+
   }
 }
 
@@ -177,5 +203,9 @@ void RIFF::File::writeChunk(const ByteVector &name, const ByteVector &data,
   ByteVector combined = name;
   combined.append(ByteVector::fromUInt(data.size(), d->endianness == BigEndian));
   combined.append(data);
+  if((data.size() & 0x01) != 0) {
+    // padding
+    combined.append('\x00');
+  }
   insert(combined, offset, replace);
 }

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 6afefe6da91eb8d137cf4f0e3fbba28e1384f599..41b1129f803650b12e0958f941b4b79434e7c216 100644 (file)
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -9,6 +9,8 @@ INCLUDE_DIRECTORIES(
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg/id3v2/frames
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mpeg
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/mp4
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/riff
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/riff/aiff
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/trueaudio
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ogg
 )
@@ -27,6 +29,8 @@ SET(test_runner_SRCS
   test_id3v1.cpp
   test_id3v2.cpp
   test_xiphcomment.cpp
+  test_aiff.cpp
+  test_riff.cpp
 )
 IF(WITH_MP4)
    SET(test_runner_SRCS ${test_runner_SRCS} test_mp4.cpp)

diff --git a/tests/data/empty.aiff b/tests/data/empty.aiff
new file mode 100644 (file)
index 0000000..849b762
Binary files /dev/null and b/tests/data/empty.aiff differ

diff --git a/tests/test_aiff.cpp b/tests/test_aiff.cpp
new file mode 100644 (file)
index 0000000..be94b07
--- /dev/null
+++ b/tests/test_aiff.cpp
@@ -0,0 +1,32 @@
+#include <cppunit/extensions/HelperMacros.h>
+#include <string>
+#include <stdio.h>
+#include <tag.h>
+#include <tbytevectorlist.h>
+#include <aifffile.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+class TestAIFF : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestAIFF);
+  CPPUNIT_TEST(testReading);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+
+  void testReading()
+  {
+    string filename = copyFile("empty", ".aiff");
+
+    RIFF::AIFF::File *f = new RIFF::AIFF::File(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(689, f->audioProperties()->bitrate());
+
+    deleteFile(filename);
+  }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestAIFF);

diff --git a/tests/test_riff.cpp b/tests/test_riff.cpp
new file mode 100644 (file)
index 0000000..34ab84e
--- /dev/null
+++ b/tests/test_riff.cpp
@@ -0,0 +1,81 @@
+#include <cppunit/extensions/HelperMacros.h>
+#include <string>
+#include <stdio.h>
+#include <tag.h>
+#include <tbytevectorlist.h>
+#include <rifffile.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+class PublicRIFF : public RIFF::File
+{
+public:
+  PublicRIFF(FileName file) : RIFF::File(file, BigEndian) {};
+  TagLib::uint chunkCount() { return RIFF::File::chunkCount(); };
+  TagLib::uint chunkOffset(TagLib::uint i) { return RIFF::File::chunkOffset(i); };
+  ByteVector chunkName(TagLib::uint i) { return RIFF::File::chunkName(i); };
+  ByteVector chunkData(TagLib::uint i) { return RIFF::File::chunkData(i); };
+  void setChunkData(const ByteVector &name, const ByteVector &data) {
+    RIFF::File::setChunkData(name, data);
+  };
+  virtual TagLib::Tag* tag() const { return 0; };
+  virtual TagLib::AudioProperties* audioProperties() const { return 0;};
+  virtual bool save() { return false; };
+};
+
+class TestRIFF : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestRIFF);
+  CPPUNIT_TEST(testPadding);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+
+  void testPadding()
+  {
+    string filename = copyFile("empty", ".aiff");
+
+    PublicRIFF *f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0x1728 + 8), f->chunkOffset(2));
+
+    f->setChunkData("TEST", "foo");
+    delete f;
+
+    f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("foo"), f->chunkData(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0x1728 + 8), f->chunkOffset(2));
+
+    f->setChunkData("SSND", "abcd");
+
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(1));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("abcd"), f->chunkData(1));
+
+    f->seek(f->chunkOffset(1));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("abcd"), f->readBlock(4));
+
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("foo"), f->chunkData(2));
+
+    f->seek(f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("foo"), f->readBlock(3));
+
+    delete f;
+
+    f = new PublicRIFF(filename.c_str());
+
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(1));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("abcd"), f->chunkData(1));
+
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("foo"), f->chunkData(2));
+
+    deleteFile(filename);
+  }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestRIFF);