From d3e79ddc389bf79ece829e5412cbbb8a5227ed6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sun, 13 Mar 2011 21:33:57 +0100 Subject: [PATCH 1/3] Partial protection against broken WMA files This fixes the problem on the reported file, but in general this code needs a lot more checks. https://bugs.kde.org/show_bug.cgi?id=268401 --- taglib/asf/asffile.cpp | 70 ++++++++++++++++++++++++++++++++++++------ taglib/asf/asffile.h | 8 ++--- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/taglib/asf/asffile.cpp b/taglib/asf/asffile.cpp index 0a947472..e39f856f 100644 --- a/taglib/asf/asffile.cpp +++ b/taglib/asf/asffile.cpp @@ -319,7 +319,16 @@ void ASF::File::HeaderExtensionObject::parse(ASF::File *file, uint /*size*/) long long dataPos = 0; while(dataPos < dataSize) { ByteVector guid = file->readBlock(16); - long long size = file->readQWORD(); + if(guid.size() != 16) { + file->setValid(false); + break; + } + bool ok; + long long size = file->readQWORD(&ok); + if(!ok) { + file->setValid(false); + break; + } BaseObject *obj; if(guid == metadataGuid) { obj = new MetadataObject(); @@ -389,19 +398,37 @@ void ASF::File::read(bool /*readProperties*/, Properties::ReadStyle /*properties ByteVector guid = readBlock(16); if(guid != headerGuid) { debug("ASF: Not an ASF file."); + setValid(false); return; } d->tag = new ASF::Tag(); d->properties = new ASF::Properties(); - d->size = readQWORD(); - int numObjects = readDWORD(); + bool ok; + d->size = readQWORD(&ok); + if(!ok) { + setValid(false); + return; + } + int numObjects = readDWORD(&ok); + if(!ok) { + setValid(false); + return; + } seek(2, Current); for(int i = 0; i < numObjects; i++) { ByteVector guid = readBlock(16); - long size = (long)readQWORD(); + if(guid.size() != 16) { + setValid(false); + break; + } + long size = (long)readQWORD(&ok); + if(!ok) { + setValid(false); + break; + } BaseObject *obj; if(guid == filePropertiesGuid) { obj = new FilePropertiesObject(); @@ -429,7 +456,12 @@ void ASF::File::read(bool /*readProperties*/, Properties::ReadStyle /*properties bool ASF::File::save() { if(readOnly()) { - debug("ASF: File is read-only."); + debug("ASF::File::save() -- File is read only."); + return false; + } + + if(!isValid()) { + debug("ASF::File::save() -- Trying to save invalid file."); return false; } @@ -491,27 +523,47 @@ bool ASF::File::save() // protected members //////////////////////////////////////////////////////////////////////////////// -int ASF::File::readBYTE() +int ASF::File::readBYTE(bool *ok) { ByteVector v = readBlock(1); + if(v.size() != 1) { + if(ok) *ok = false; + return 0; + } + if(ok) *ok = true; return v[0]; } -int ASF::File::readWORD() +int ASF::File::readWORD(bool *ok) { ByteVector v = readBlock(2); + if(v.size() != 2) { + if(ok) *ok = false; + return 0; + } + if(ok) *ok = true; return v.toUShort(false); } -unsigned int ASF::File::readDWORD() +unsigned int ASF::File::readDWORD(bool *ok) { ByteVector v = readBlock(4); + if(v.size() != 4) { + if(ok) *ok = false; + return 0; + } + if(ok) *ok = true; return v.toUInt(false); } -long long ASF::File::readQWORD() +long long ASF::File::readQWORD(bool *ok) { ByteVector v = readBlock(8); + if(v.size() != 8) { + if(ok) *ok = false; + return 0; + } + if(ok) *ok = true; return v.toLongLong(false); } diff --git a/taglib/asf/asffile.h b/taglib/asf/asffile.h index 9242aa68..45e603dc 100644 --- a/taglib/asf/asffile.h +++ b/taglib/asf/asffile.h @@ -88,10 +88,10 @@ namespace TagLib { private: - int readBYTE(); - int readWORD(); - unsigned int readDWORD(); - long long readQWORD(); + int readBYTE(bool *ok = 0); + int readWORD(bool *ok = 0); + unsigned int readDWORD(bool *ok = 0); + long long readQWORD(bool *ok = 0); static ByteVector renderString(const String &str, bool includeLength = false); String readString(int len); void read(bool readProperties, Properties::ReadStyle propertiesStyle); From f624d6e2af282c1b8696e4d7eb24394b338efc82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sat, 19 Mar 2011 07:37:28 +0100 Subject: [PATCH 2/3] Don't overwrite fields that already exist We can have multiple fields in the Vorbis Comment (e.g. two artist names), but TagUnion only takes the first one, so it will effectively strip the extra fields. https://bugs.kde.org/show_bug.cgi?id=268854 --- taglib/flac/flacfile.cpp | 2 +- tests/test_flac.cpp | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index c8cc1fb8..f882ae7b 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -149,7 +149,7 @@ bool FLAC::File::save() // Create new vorbis comments - Tag::duplicate(&d->tag, xiphComment(true), true); + Tag::duplicate(&d->tag, xiphComment(true), false); d->xiphCommentData = xiphComment()->render(false); diff --git a/tests/test_flac.cpp b/tests/test_flac.cpp index ba3d99da..4b54a7ca 100644 --- a/tests/test_flac.cpp +++ b/tests/test_flac.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include "utils.h" using namespace std; @@ -20,6 +21,7 @@ class TestFLAC : public CppUnit::TestFixture CPPUNIT_TEST(testReplacePicture); CPPUNIT_TEST(testRemoveAllPictures); CPPUNIT_TEST(testRepeatedSave); + CPPUNIT_TEST(testSaveMultipleValues); CPPUNIT_TEST_SUITE_END(); public: @@ -186,6 +188,26 @@ public: CPPUNIT_ASSERT_EQUAL(String("NEW TITLE 2"), tag->title()); } + void testSaveMultipleValues() + { + ScopedFileCopy copy("silence-44-s", ".flac", false); + string newname = copy.fileName(); + + FLAC::File *f = new FLAC::File(newname.c_str()); + Ogg::XiphComment* c = f->xiphComment(true); + c->addField("ARTIST", "artist 1", true); + c->addField("ARTIST", "artist 2", false); + f->save(); + delete f; + + f = new FLAC::File(newname.c_str()); + c = f->xiphComment(true); + Ogg::FieldListMap m = c->fieldListMap(); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(2), m["ARTIST"].size()); + CPPUNIT_ASSERT_EQUAL(String("artist 1"), m["ARTIST"][0]); + CPPUNIT_ASSERT_EQUAL(String("artist 2"), m["ARTIST"][1]); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestFLAC); From d112a6819390bbe322d5c6e7bf19e7862de9f5f2 Mon Sep 17 00:00:00 2001 From: Modestas Vainius Date: Sat, 9 Apr 2011 19:15:46 +0200 Subject: [PATCH 3/3] Support building documentation out-of-source-dir --- CMakeLists.txt | 3 ++- Doxyfile.cmake | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5294fae6..e0941d88 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -76,6 +76,7 @@ endif(NOT WIN32) INSTALL( PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/taglib-config DESTINATION ${BIN_INSTALL_DIR}) -CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.cmake ${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile) +CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.cmake ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile) +file(COPY doc/taglib.png DESTINATION doc) ADD_CUSTOM_TARGET(docs doxygen) diff --git a/Doxyfile.cmake b/Doxyfile.cmake index 7fdf4123..6da30bb5 100644 --- a/Doxyfile.cmake +++ b/Doxyfile.cmake @@ -61,7 +61,7 @@ WARN_LOGFILE = #--------------------------------------------------------------------------- # configuration options related to the input files #--------------------------------------------------------------------------- -INPUT = taglib +INPUT = @CMAKE_SOURCE_DIR@/taglib FILE_PATTERNS = *.h \ *.hh \ *.H @@ -96,9 +96,9 @@ IGNORE_PREFIX = GENERATE_HTML = YES HTML_OUTPUT = html HTML_FILE_EXTENSION = .html -HTML_HEADER = doc/api-header.html -HTML_FOOTER = doc/api-footer.html -HTML_STYLESHEET = doc/taglib-api.css +HTML_HEADER = @CMAKE_SOURCE_DIR@/doc/api-header.html +HTML_FOOTER = @CMAKE_SOURCE_DIR@/doc/api-footer.html +HTML_STYLESHEET = @CMAKE_SOURCE_DIR@/doc/taglib-api.css HTML_ALIGN_MEMBERS = YES GENERATE_HTMLHELP = NO CHM_FILE =