From 7bbf5a2e79389026895870566196b5e13f224f72 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 14:06:49 -0500 Subject: [PATCH 01/15] add base64 encoder/decoder to bytevector --- taglib/toolkit/tbytevector.cpp | 92 ++++++++++++++++++++++++++++++++++ taglib/toolkit/tbytevector.h | 13 +++++ tests/test_bytevector.cpp | 40 +++++++++++++++ 3 files changed, 145 insertions(+) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 6262bec6..eb559390 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -950,6 +950,98 @@ ByteVector ByteVector::toHex() const return encoded; } + + + + +ByteVector & ByteVector::fromBase64() +{ + static const unsigned char base64[256]={ + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x3e,0x80,0x80,0x80,0x3f, + 0x34,0x35,0x36,0x37,0x38,0x39,0x3a,0x3b,0x3c,0x3d,0x80,0x80,0x80,0xff,0x80,0x80, + 0x80,0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e, + 0x0f,0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17,0x18,0x19,0x80,0x80,0x80,0x80,0x80, + 0x80,0x1a,0x1b,0x1c,0x1d,0x1e,0x1f,0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,0x28, + 0x29,0x2a,0x2b,0x2c,0x2d,0x2e,0x2f,0x30,0x31,0x32,0x33,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80 + }; + + detach(); + uint len = size(); + const unsigned char * src = (unsigned char*) data(); + unsigned char * dst = (unsigned char*) data(); + while(4<=len) { + if(base64[src[0]]==0x80) break; + if(base64[src[1]]==0x80) break; + if(base64[src[2]]==0x80) break; + if(base64[src[3]]==0x80) break; + *dst++=((base64[src[0]]<<2)&0xfc)|((base64[src[1]]>>4)&0x03); + if(src[2]!='=') { + *dst++=((base64[src[1]]&0x0f)<<4)|((base64[src[2]]>>2)&0x0f); + if(src[3]!='=') { + *dst++=((base64[src[2]]&0x03)<<6)|(base64[src[3]]&0x3f); + } + else { + break; + } + } + else { + break; + } + src+=4; + len-=4; + } + resize(dst-(unsigned char*)data()); + return *this; +} + + + + +ByteVector ByteVector::toBase64() const +{ + static const char alphabet[]="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + if (size()) { + uint len=size(); + ByteVector output(4 * ((len-1)/3+1)); // note roundup + + const char * src = data(); + char * dst = output.data(); + while(3<=len) { + *dst++=alphabet[(src[0]>>2)&0x3f]; + *dst++=alphabet[((src[0]&0x03)<<4)|((src[1]>>4)&0x0f)]; + *dst++=alphabet[((src[1]&0x0f)<<2)|((src[2]>>6)&0x03)]; + *dst++=alphabet[src[2]&0x3f]; + src+=3; + len-=3; + } + if(len) { + *dst++=alphabet[(src[0]>>2)&0x3f]; + if(len>1) { + *dst++=alphabet[((src[0]&0x03)<<4)|((src[1]>>4)&0x0f)]; + *dst++=alphabet[((src[1]&0x0f)<<2)]; + } + else { + *dst++=alphabet[(src[0]&0x03)<<4]; + *dst++='='; + } + *dst++='='; + } + return output; + } + return ByteVector(); +} + + //////////////////////////////////////////////////////////////////////////////// // protected members //////////////////////////////////////////////////////////////////////////////// diff --git a/taglib/toolkit/tbytevector.h b/taglib/toolkit/tbytevector.h index 793b414c..0f96c77f 100644 --- a/taglib/toolkit/tbytevector.h +++ b/taglib/toolkit/tbytevector.h @@ -573,6 +573,19 @@ namespace TagLib { */ ByteVector toHex() const; + /*! + * Returns a base64 encoded copy of the byte vector + */ + ByteVector toBase64() const; + + /*! + * Decodes the base64 encoded byte vector in-memory. Returns a reference + * to this vector. Calls detach before decoding. + */ + ByteVector & fromBase64(); + + + protected: /* * If this ByteVector is being shared via implicit sharing, do a deep copy diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 1c2ca904..19f51518 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -43,6 +43,7 @@ class TestByteVector : public CppUnit::TestFixture CPPUNIT_TEST(testReplace); CPPUNIT_TEST(testIterator); CPPUNIT_TEST(testResize); + CPPUNIT_TEST(testBase64); CPPUNIT_TEST_SUITE_END(); public: @@ -379,6 +380,45 @@ public: CPPUNIT_ASSERT_EQUAL(-1, c.find('C')); } + void testBase64() + { + ByteVector sempty; + ByteVector t0("a"); // test 1 byte + ByteVector t1("any carnal pleasure."); + ByteVector t2("any carnal pleasure"); + ByteVector t3("any carnal pleasur"); + ByteVector s0("a"); // test 1 byte + ByteVector s1("any carnal pleasure."); + ByteVector s2("any carnal pleasure"); + ByteVector s3("any carnal pleasur"); + ByteVector eempty; + ByteVector e0("YQ=="); + ByteVector e1("YW55IGNhcm5hbCBwbGVhc3VyZS4="); + ByteVector e2("YW55IGNhcm5hbCBwbGVhc3VyZQ=="); + ByteVector e3("YW55IGNhcm5hbCBwbGVhc3Vy"); + + // Encode + CPPUNIT_ASSERT_EQUAL(eempty, sempty.toBase64()); + CPPUNIT_ASSERT_EQUAL(e0, s0.toBase64()); + CPPUNIT_ASSERT_EQUAL(e1, s1.toBase64()); + CPPUNIT_ASSERT_EQUAL(e2, s2.toBase64()); + CPPUNIT_ASSERT_EQUAL(e3, s3.toBase64()); + + // Decode + CPPUNIT_ASSERT_EQUAL(sempty, eempty.toBase64()); + CPPUNIT_ASSERT_EQUAL(s0, e0.fromBase64()); + CPPUNIT_ASSERT_EQUAL(s1, e1.fromBase64()); + CPPUNIT_ASSERT_EQUAL(s2, e2.fromBase64()); + CPPUNIT_ASSERT_EQUAL(s3, e3.fromBase64()); + + CPPUNIT_ASSERT_EQUAL(t0, s0.toBase64().fromBase64()); + CPPUNIT_ASSERT_EQUAL(t1, s1.toBase64().fromBase64()); + CPPUNIT_ASSERT_EQUAL(t2, s2.toBase64().fromBase64()); + CPPUNIT_ASSERT_EQUAL(t3, s3.toBase64().fromBase64()); + + } + + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestByteVector); From 0c14bd0ed075d140ef55fc62efc81b8c3ad841c8 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 14:10:24 -0500 Subject: [PATCH 02/15] Add FLAC::Picture support to Xiph Comment --- taglib/ogg/xiphcomment.cpp | 87 +++++++++++++++++++++++++++++++++----- taglib/ogg/xiphcomment.h | 26 ++++++++++++ tests/test_ogg.cpp | 28 ++++++++++++ 3 files changed, 130 insertions(+), 11 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 9462607f..c307e618 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -26,17 +26,22 @@ #include #include +#include #include #include using namespace TagLib; + +typedef List PictureList; + class Ogg::XiphComment::XiphCommentPrivate { public: FieldListMap fieldListMap; String vendorID; String commentField; + PictureList pictureList; }; //////////////////////////////////////////////////////////////////////////////// @@ -181,6 +186,8 @@ TagLib::uint Ogg::XiphComment::fieldCount() const for(; it != d->fieldListMap.end(); ++it) count += (*it).second.size(); + count += d->pictureList.size(); + return count; } @@ -275,6 +282,36 @@ bool Ogg::XiphComment::contains(const String &key) const return d->fieldListMap.contains(key) && !d->fieldListMap[key].isEmpty(); } +void Ogg::XiphComment::removePicture(FLAC::Picture *picture, bool del) +{ + List::Iterator it = d->pictureList.find(picture); + if(it != d->pictureList.end()) + d->pictureList.erase(it); + + if(del) + delete picture; +} + +void Ogg::XiphComment::removePictures() +{ + PictureList newList; + for(uint i = 0; i < d->pictureList.size(); i++) { + delete d->pictureList[i]; + } + d->pictureList = newList; +} + +void Ogg::XiphComment::addPicture(FLAC::Picture * picture) +{ + d->pictureList.append(picture); +} + + +List Ogg::XiphComment::pictureList() +{ + return d->pictureList; +} + ByteVector Ogg::XiphComment::render() const { return render(true); @@ -321,6 +358,13 @@ ByteVector Ogg::XiphComment::render(bool addFramingBit) const } } + for(PictureList::ConstIterator it = d->pictureList.begin(); it != d->pictureList.end(); ++it) { + ByteVector picture = (*it)->render().toBase64(); + data.append(ByteVector::fromUInt(picture.size()+23,false)); + data.append("METADATA_BLOCK_PICTURE="); + data.append(picture); + } + // Append the "framing bit". if(addFramingBit) @@ -363,20 +407,41 @@ void Ogg::XiphComment::parse(const ByteVector &data) const uint commentLength = data.toUInt(pos, false); pos += 4; - String comment = String(data.mid(pos, commentLength), String::UTF8); - pos += commentLength; - if(pos > data.size()) { + ByteVector entry = data.mid(pos, commentLength); + + // Don't go past data end + pos+=commentLength; + if (pos>data.size()) break; + + // Handle Pictures separately + if(entry.startsWith("METADATA_BLOCK_PICTURE=")) { + + // Decode base64 picture data + ByteVector picturedata = entry.mid(23, entry.size()-23).fromBase64(); + + if(picturedata.size()==0) { + debug("Empty picture data. Discarding content"); + continue; + } + + FLAC::Picture * picture = new FLAC::Picture(); + if(picture->parse(picturedata)) + d->pictureList.append(picture); + else + debug("Unable to parse METADATA_BLOCK_PICTURE. Discarding content."); } + else { - int commentSeparatorPosition = comment.find("="); - if(commentSeparatorPosition == -1) { - break; + // Check for field separator + int sep = entry.find('='); + if (sep == -1) + break; + + // Parse key and value + String key = String(entry.mid(0,sep), String::UTF8); + String value = String(entry.mid(sep+1, commentLength-sep), String::UTF8); + addField(key, value, false); } - - String key = comment.substr(0, commentSeparatorPosition); - String value = comment.substr(commentSeparatorPosition + 1); - - addField(key, value, false); } } diff --git a/taglib/ogg/xiphcomment.h b/taglib/ogg/xiphcomment.h index 54f3070b..8fcc6873 100644 --- a/taglib/ogg/xiphcomment.h +++ b/taglib/ogg/xiphcomment.h @@ -32,6 +32,7 @@ #include "tstring.h" #include "tstringlist.h" #include "tbytevector.h" +#include "flacpicture.h" #include "taglib_export.h" namespace TagLib { @@ -205,6 +206,31 @@ namespace TagLib { */ ByteVector render(bool addFramingBit) const; + + /*! + * Returns a list of pictures attached to the xiph comment. + */ + List pictureList(); + + /*! + * Removes an picture. If \a del is true the picture's memory + * will be freed; if it is false, it must be deleted by the user. + */ + void removePicture(FLAC::Picture *picture, bool del = true); + + /*! + * Remove all pictures. + */ + void removePictures(); + + /*! + * Add a new picture to the comment block. The comment block takes ownership of the + * picture and will handle freeing its memory. + * + * \note The file will be saved only after calling save(). + */ + void addPicture(FLAC::Picture *picture); + protected: /*! * Reads the tag from the file specified in the constructor and fills the diff --git a/tests/test_ogg.cpp b/tests/test_ogg.cpp index 210fca2f..05b23f64 100644 --- a/tests/test_ogg.cpp +++ b/tests/test_ogg.cpp @@ -21,6 +21,7 @@ class TestOGG : public CppUnit::TestFixture CPPUNIT_TEST(testDictInterface1); CPPUNIT_TEST(testDictInterface2); CPPUNIT_TEST(testAudioProperties); + CPPUNIT_TEST(testPicture); CPPUNIT_TEST_SUITE_END(); public: @@ -134,6 +135,33 @@ public: CPPUNIT_ASSERT_EQUAL(112000, f.audioProperties()->bitrateNominal()); CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrateMinimum()); } + + void testPicture() + { + ScopedFileCopy copy("empty", ".ogg"); + string newname = copy.fileName(); + + Vorbis::File *f = new Vorbis::File(newname.c_str()); + FLAC::Picture *newpic = new FLAC::Picture(); + newpic->setType(FLAC::Picture::BackCover); + newpic->setWidth(5); + newpic->setHeight(6); + newpic->setColorDepth(16); + newpic->setNumColors(7); + newpic->setMimeType("image/jpeg"); + newpic->setDescription("new image"); + newpic->setData("JPEG data"); + f->tag()->addPicture(newpic); + f->save(); + delete f; + + f = new Vorbis::File(newname.c_str()); + List lst = f->tag()->pictureList(); + CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), lst.size()); + delete f; + } + + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestOGG); From d3351a0012d89bd78e032859597e4f4ef40efdd5 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 14:14:06 -0500 Subject: [PATCH 03/15] Delete pictures in XiphComment destructor --- taglib/ogg/xiphcomment.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index c307e618..2783d922 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -61,6 +61,7 @@ Ogg::XiphComment::XiphComment(const ByteVector &data) : TagLib::Tag() Ogg::XiphComment::~XiphComment() { + removePictures(); delete d; } From 62bb3c95c80e6c1646a54cb3b96bc01bdebe4ff2 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 14:51:10 -0500 Subject: [PATCH 04/15] Make ByteVector::fromBase64 as static member function --- taglib/ogg/xiphcomment.cpp | 2 +- taglib/toolkit/tbytevector.cpp | 16 +++++++++------- taglib/toolkit/tbytevector.h | 7 ++----- tests/test_bytevector.cpp | 16 ++++++++-------- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 2783d922..15f8cbaf 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -419,7 +419,7 @@ void Ogg::XiphComment::parse(const ByteVector &data) if(entry.startsWith("METADATA_BLOCK_PICTURE=")) { // Decode base64 picture data - ByteVector picturedata = entry.mid(23, entry.size()-23).fromBase64(); + ByteVector picturedata = ByteVector::fromBase64(entry.mid(23)); if(picturedata.size()==0) { debug("Empty picture data. Discarding content"); diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index eb559390..f18d2329 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -954,7 +954,7 @@ ByteVector ByteVector::toHex() const -ByteVector & ByteVector::fromBase64() +ByteVector ByteVector::fromBase64(const ByteVector & input) { static const unsigned char base64[256]={ 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, @@ -975,10 +975,12 @@ ByteVector & ByteVector::fromBase64() 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80 }; - detach(); - uint len = size(); - const unsigned char * src = (unsigned char*) data(); - unsigned char * dst = (unsigned char*) data(); + uint len = input.size(); + + ByteVector output(len); + + const unsigned char * src = (unsigned char*) input.data(); + unsigned char * dst = (unsigned char*)output.data(); while(4<=len) { if(base64[src[0]]==0x80) break; if(base64[src[1]]==0x80) break; @@ -1000,8 +1002,8 @@ ByteVector & ByteVector::fromBase64() src+=4; len-=4; } - resize(dst-(unsigned char*)data()); - return *this; + output.resize(dst-(unsigned char*)output.data()); + return output; } diff --git a/taglib/toolkit/tbytevector.h b/taglib/toolkit/tbytevector.h index 0f96c77f..4b669b9e 100644 --- a/taglib/toolkit/tbytevector.h +++ b/taglib/toolkit/tbytevector.h @@ -579,12 +579,9 @@ namespace TagLib { ByteVector toBase64() const; /*! - * Decodes the base64 encoded byte vector in-memory. Returns a reference - * to this vector. Calls detach before decoding. + * Decodes the base64 encoded byte vector. */ - ByteVector & fromBase64(); - - + static ByteVector fromBase64(const ByteVector &); protected: /* diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 19f51518..18097adf 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -406,15 +406,15 @@ public: // Decode CPPUNIT_ASSERT_EQUAL(sempty, eempty.toBase64()); - CPPUNIT_ASSERT_EQUAL(s0, e0.fromBase64()); - CPPUNIT_ASSERT_EQUAL(s1, e1.fromBase64()); - CPPUNIT_ASSERT_EQUAL(s2, e2.fromBase64()); - CPPUNIT_ASSERT_EQUAL(s3, e3.fromBase64()); + CPPUNIT_ASSERT_EQUAL(s0, ByteVector::fromBase64(e0)); + CPPUNIT_ASSERT_EQUAL(s1, ByteVector::fromBase64(e1)); + CPPUNIT_ASSERT_EQUAL(s2, ByteVector::fromBase64(e2)); + CPPUNIT_ASSERT_EQUAL(s3, ByteVector::fromBase64(e3)); - CPPUNIT_ASSERT_EQUAL(t0, s0.toBase64().fromBase64()); - CPPUNIT_ASSERT_EQUAL(t1, s1.toBase64().fromBase64()); - CPPUNIT_ASSERT_EQUAL(t2, s2.toBase64().fromBase64()); - CPPUNIT_ASSERT_EQUAL(t3, s3.toBase64().fromBase64()); + CPPUNIT_ASSERT_EQUAL(t0, ByteVector::fromBase64(s0.toBase64())); + CPPUNIT_ASSERT_EQUAL(t1, ByteVector::fromBase64(s1.toBase64())); + CPPUNIT_ASSERT_EQUAL(t2, ByteVector::fromBase64(s2.toBase64())); + CPPUNIT_ASSERT_EQUAL(t3, ByteVector::fromBase64(s3.toBase64())); } From 59a1b7a4916e3cf47945ba965961249db21fb9db Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 14:53:17 -0500 Subject: [PATCH 05/15] add forgotten const cast --- taglib/toolkit/tbytevector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index f18d2329..a87922d5 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -979,7 +979,7 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) ByteVector output(len); - const unsigned char * src = (unsigned char*) input.data(); + const unsigned char * src = (const unsigned char*) input.data(); unsigned char * dst = (unsigned char*)output.data(); while(4<=len) { if(base64[src[0]]==0x80) break; From c4a02a17991a857ad01d9ed61aad7da167faec4e Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 14:57:19 -0500 Subject: [PATCH 06/15] remove redundant size specificier in mid usage --- taglib/ogg/xiphcomment.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 15f8cbaf..431c7980 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -441,7 +441,7 @@ void Ogg::XiphComment::parse(const ByteVector &data) // Parse key and value String key = String(entry.mid(0,sep), String::UTF8); - String value = String(entry.mid(sep+1, commentLength-sep), String::UTF8); + String value = String(entry.mid(sep+1), String::UTF8); addField(key, value, false); } } From c963d3ba04d0cd4ad227c2703d978c023d659833 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 15:05:05 -0500 Subject: [PATCH 07/15] correctly filter out invalid base64 characters --- taglib/toolkit/tbytevector.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index a87922d5..377dc034 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -960,7 +960,7 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x3e,0x80,0x80,0x80,0x3f, - 0x34,0x35,0x36,0x37,0x38,0x39,0x3a,0x3b,0x3c,0x3d,0x80,0x80,0x80,0xff,0x80,0x80, + 0x34,0x35,0x36,0x37,0x38,0x39,0x3a,0x3b,0x3c,0x3d,0x80,0x80,0x80,0x80,0x80,0x80, 0x80,0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e, 0x0f,0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17,0x18,0x19,0x80,0x80,0x80,0x80,0x80, 0x80,0x1a,0x1b,0x1c,0x1d,0x1e,0x1f,0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,0x28, @@ -984,12 +984,12 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) while(4<=len) { if(base64[src[0]]==0x80) break; if(base64[src[1]]==0x80) break; - if(base64[src[2]]==0x80) break; - if(base64[src[3]]==0x80) break; *dst++=((base64[src[0]]<<2)&0xfc)|((base64[src[1]]>>4)&0x03); if(src[2]!='=') { + if(base64[src[2]]==0x80) break; *dst++=((base64[src[1]]&0x0f)<<4)|((base64[src[2]]>>2)&0x0f); if(src[3]!='=') { + if(base64[src[3]]==0x80) break; *dst++=((base64[src[2]]&0x03)<<6)|(base64[src[3]]&0x3f); } else { From f4c1beb16147dee202824ff693d55da69eb110be Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 21:25:06 -0500 Subject: [PATCH 08/15] Make fromBase64 more strict and update code to follow taglib coding style --- taglib/ogg/xiphcomment.cpp | 22 ++++---- taglib/toolkit/tbytevector.cpp | 99 +++++++++++++++++++++------------- 2 files changed, 74 insertions(+), 47 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 431c7980..6b87e3ea 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -33,7 +33,7 @@ using namespace TagLib; -typedef List PictureList; +typedef List PictureList; class Ogg::XiphComment::XiphCommentPrivate { @@ -285,7 +285,7 @@ bool Ogg::XiphComment::contains(const String &key) const void Ogg::XiphComment::removePicture(FLAC::Picture *picture, bool del) { - List::Iterator it = d->pictureList.find(picture); + PictureList::Iterator it = d->pictureList.find(picture); if(it != d->pictureList.end()) d->pictureList.erase(it); @@ -410,9 +410,10 @@ void Ogg::XiphComment::parse(const ByteVector &data) ByteVector entry = data.mid(pos, commentLength); + pos += commentLength; + // Don't go past data end - pos+=commentLength; - if (pos>data.size()) + if (pos > data.size()) break; // Handle Pictures separately @@ -421,12 +422,13 @@ void Ogg::XiphComment::parse(const ByteVector &data) // Decode base64 picture data ByteVector picturedata = ByteVector::fromBase64(entry.mid(23)); - if(picturedata.size()==0) { + if(picturedata.size() == 0) { debug("Empty picture data. Discarding content"); continue; - } + } FLAC::Picture * picture = new FLAC::Picture(); + if(picture->parse(picturedata)) d->pictureList.append(picture); else @@ -436,11 +438,13 @@ void Ogg::XiphComment::parse(const ByteVector &data) // Check for field separator int sep = entry.find('='); - if (sep == -1) - break; + if (sep < 1) { + debug("Discarding invalid comment field."); + continue; + } // Parse key and value - String key = String(entry.mid(0,sep), String::UTF8); + String key = String(entry.mid(0, sep), String::UTF8); String value = String(entry.mid(sep+1), String::UTF8); addField(key, value, false); } diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 377dc034..14550ad1 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -950,13 +950,9 @@ ByteVector ByteVector::toHex() const return encoded; } - - - - ByteVector ByteVector::fromBase64(const ByteVector & input) { - static const unsigned char base64[256]={ + static const unsigned char base64[256] = { 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x3e,0x80,0x80,0x80,0x3f, @@ -980,61 +976,88 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) ByteVector output(len); const unsigned char * src = (const unsigned char*) input.data(); - unsigned char * dst = (unsigned char*)output.data(); - while(4<=len) { - if(base64[src[0]]==0x80) break; - if(base64[src[1]]==0x80) break; - *dst++=((base64[src[0]]<<2)&0xfc)|((base64[src[1]]>>4)&0x03); - if(src[2]!='=') { - if(base64[src[2]]==0x80) break; - *dst++=((base64[src[1]]&0x0f)<<4)|((base64[src[2]]>>2)&0x0f); - if(src[3]!='=') { - if(base64[src[3]]==0x80) break; - *dst++=((base64[src[2]]&0x03)<<6)|(base64[src[3]]&0x3f); + unsigned char * dst = (unsigned char*) output.data(); + + while(4 <= len) { + + // Check invalid character + if(base64[src[0]] == 0x80) + break; + + // Check invalid character + if(base64[src[1]] == 0x80) + break; + + // Decode first byte + *dst++ = ((base64[src[0]] << 2) & 0xfc) | ((base64[src[1]] >> 4) & 0x03); + + if(src[2] != '=') { + + // Check invalid character + if(base64[src[2]] == 0x80) + break; + + // Decode second byte + *dst++ = ((base64[src[1]] & 0x0f) << 4) | ((base64[src[2]] >> 2) & 0x0f); + + if(src[3] != '=') { + + // Check invalid character + if(base64[src[3]] == 0x80) + break; + + // Decode third byte + *dst++ = ((base64[src[2]] & 0x03) << 6) | (base64[src[3]] & 0x3f); } else { + // assume end of data + len -= 4; break; } } else { + // assume end of data + len -= 4; break; } - src+=4; - len-=4; + src += 4; + len -= 4; } - output.resize(dst-(unsigned char*)output.data()); - return output; + + // Only return output if we processed all bytes + if(len == 0) { + output.resize(dst - (unsigned char*) output.data()); + return output; + } + return ByteVector(); } - - - ByteVector ByteVector::toBase64() const { - static const char alphabet[]="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + static const char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; if (size()) { - uint len=size(); - ByteVector output(4 * ((len-1)/3+1)); // note roundup + uint len = size(); + ByteVector output(4 * ((len - 1) / 3 + 1)); // note roundup const char * src = data(); char * dst = output.data(); - while(3<=len) { - *dst++=alphabet[(src[0]>>2)&0x3f]; - *dst++=alphabet[((src[0]&0x03)<<4)|((src[1]>>4)&0x0f)]; - *dst++=alphabet[((src[1]&0x0f)<<2)|((src[2]>>6)&0x03)]; - *dst++=alphabet[src[2]&0x3f]; - src+=3; - len-=3; + while(3 <= len) { + *dst++ = alphabet[(src[0] >> 2) & 0x3f]; + *dst++ = alphabet[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0f)]; + *dst++ = alphabet[((src[1] & 0x0f) << 2) | ((src[2] >> 6) & 0x03)]; + *dst++ = alphabet[src[2] & 0x3f]; + src += 3; + len -= 3; } if(len) { - *dst++=alphabet[(src[0]>>2)&0x3f]; + *dst++ = alphabet[(src[0] >> 2) & 0x3f]; if(len>1) { - *dst++=alphabet[((src[0]&0x03)<<4)|((src[1]>>4)&0x0f)]; - *dst++=alphabet[((src[1]&0x0f)<<2)]; + *dst++ = alphabet[((src[0] & 0x03) << 4)|((src[1] >> 4) & 0x0f)]; + *dst++ = alphabet[((src[1] & 0x0f) << 2)]; } else { - *dst++=alphabet[(src[0]&0x03)<<4]; - *dst++='='; + *dst++ = alphabet[(src[0] & 0x03) << 4]; + *dst++ = '='; } *dst++='='; } From 90ad17b4c5da178d91c9a20e8c3724ea9e57de53 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 21:26:30 -0500 Subject: [PATCH 09/15] Additional unit tests for base64 encoder/decoder --- tests/test_bytevector.cpp | 68 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 18097adf..450402e3 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include using namespace std; @@ -416,6 +417,73 @@ public: CPPUNIT_ASSERT_EQUAL(t2, ByteVector::fromBase64(s2.toBase64())); CPPUNIT_ASSERT_EQUAL(t3, ByteVector::fromBase64(s3.toBase64())); + ByteVector all((uint)256); + + // in order + { + for(int i = 0; i < 256; i++){ + all[i]=(unsigned char)i; + } + ByteVector b64 = all.toBase64(); + ByteVector original = ByteVector::fromBase64(b64); + CPPUNIT_ASSERT_EQUAL(all,original); + } + + // reverse + { + for(int i = 0; i < 256; i++){ + all[i]=(unsigned char)255-i; + } + ByteVector b64 = all.toBase64(); + ByteVector original = ByteVector::fromBase64(b64); + CPPUNIT_ASSERT_EQUAL(all,original); + } + + // all zeroes + { + for(int i = 0; i < 256; i++){ + all[i]=0; + } + ByteVector b64 = all.toBase64(); + ByteVector original = ByteVector::fromBase64(b64); + CPPUNIT_ASSERT_EQUAL(all,original); + } + + // all ones + { + for(int i = 0; i < 256; i++){ + all[i]=0xff; + } + ByteVector b64 = all.toBase64(); + ByteVector original = ByteVector::fromBase64(b64); + CPPUNIT_ASSERT_EQUAL(all,original); + } + + // Missing end bytes + { + // No missing bytes + ByteVector m0("YW55IGNhcm5hbCBwbGVhc3VyZQ=="); + CPPUNIT_ASSERT_EQUAL(s2,ByteVector::fromBase64(m0)); + + // 1 missing byte + ByteVector m1("YW55IGNhcm5hbCBwbGVhc3VyZQ="); + CPPUNIT_ASSERT_EQUAL(sempty,ByteVector::fromBase64(m1)); + + // 2 missing bytes + ByteVector m2("YW55IGNhcm5hbCBwbGVhc3VyZQ"); + CPPUNIT_ASSERT_EQUAL(sempty,ByteVector::fromBase64(m2)); + + // 3 missing bytes + ByteVector m3("YW55IGNhcm5hbCBwbGVhc3VyZ"); + CPPUNIT_ASSERT_EQUAL(sempty,ByteVector::fromBase64(m3)); + } + + // Grok invalid characters + { + ByteVector invalid("abd\x00\x01\x02\x03\x04"); + CPPUNIT_ASSERT_EQUAL(sempty,ByteVector::fromBase64(invalid)); + } + } From 47bd01ecda5852070de3821cd954658e8560d389 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 21:32:50 -0500 Subject: [PATCH 10/15] Minor style fixes --- taglib/ogg/xiphcomment.cpp | 4 ++-- tests/test_bytevector.cpp | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 6b87e3ea..cb607ce4 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -361,7 +361,7 @@ ByteVector Ogg::XiphComment::render(bool addFramingBit) const for(PictureList::ConstIterator it = d->pictureList.begin(); it != d->pictureList.end(); ++it) { ByteVector picture = (*it)->render().toBase64(); - data.append(ByteVector::fromUInt(picture.size()+23,false)); + data.append(ByteVector::fromUInt(picture.size() + 23, false)); data.append("METADATA_BLOCK_PICTURE="); data.append(picture); } @@ -445,7 +445,7 @@ void Ogg::XiphComment::parse(const ByteVector &data) // Parse key and value String key = String(entry.mid(0, sep), String::UTF8); - String value = String(entry.mid(sep+1), String::UTF8); + String value = String(entry.mid(sep + 1), String::UTF8); addField(key, value, false); } } diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 450402e3..8d23b82b 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include using namespace std; From f836cb4dea012826c00c371783db2b810438e750 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 21:38:01 -0500 Subject: [PATCH 11/15] Two more spaces --- taglib/toolkit/tbytevector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 14550ad1..531503f7 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -1052,7 +1052,7 @@ ByteVector ByteVector::toBase64() const if(len) { *dst++ = alphabet[(src[0] >> 2) & 0x3f]; if(len>1) { - *dst++ = alphabet[((src[0] & 0x03) << 4)|((src[1] >> 4) & 0x0f)]; + *dst++ = alphabet[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0f)]; *dst++ = alphabet[((src[1] & 0x0f) << 2)]; } else { From 70e471f1d19a3be0caaf6000590eb48c71b439f5 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 22:09:27 -0500 Subject: [PATCH 12/15] Check if saved picture can be read back correctly --- tests/test_ogg.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_ogg.cpp b/tests/test_ogg.cpp index 05b23f64..3afeb196 100644 --- a/tests/test_ogg.cpp +++ b/tests/test_ogg.cpp @@ -158,6 +158,14 @@ public: f = new Vorbis::File(newname.c_str()); List lst = f->tag()->pictureList(); CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), lst.size()); + CPPUNIT_ASSERT_EQUAL(int(5), lst[0]->width()); + CPPUNIT_ASSERT_EQUAL(int(6), lst[0]->height()); + CPPUNIT_ASSERT_EQUAL(int(16), lst[0]->colorDepth()); + CPPUNIT_ASSERT_EQUAL(int(7), lst[0]->numColors()); + CPPUNIT_ASSERT_EQUAL(String("image/jpeg"), lst[0]->mimeType()); + CPPUNIT_ASSERT_EQUAL(String("new image"), lst[0]->description()); + CPPUNIT_ASSERT_EQUAL(ByteVector("JPEG data"), lst[0]->data()); + delete f; } From 41a44f4ab2b1dda149e54c9b7431dd8624d22c51 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 22:34:08 -0500 Subject: [PATCH 13/15] Support reading deprecated COVERART xiphcomment --- taglib/ogg/xiphcomment.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index cb607ce4..0424ed55 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -434,6 +434,23 @@ void Ogg::XiphComment::parse(const ByteVector &data) else debug("Unable to parse METADATA_BLOCK_PICTURE. Discarding content."); } + else if (entry.startsWith("COVERART=")) { + + // Decode base64 picture data + ByteVector picturedata = ByteVector::fromBase64(entry.mid(9)); + + if (picturedata.size() == 0) { + debug("Empty coverart data. Discaring content"); + continue; + } + + // Assume it's some type of image file + FLAC::Picture * picture = new FLAC::Picture(); + picture->setData(picturedata); + picture->setMimeType("image/"); + picture->setType(FLAC::Picture::Other); + d->pictureList.append(picture); + } else { // Check for field separator From 3e8eff16b8fc8de6a38fd6c159cb21990d1fc051 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Sat, 16 May 2015 22:37:54 -0500 Subject: [PATCH 14/15] another style fix --- taglib/toolkit/tbytevector.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 531503f7..757e41c3 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -981,11 +981,11 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) while(4 <= len) { // Check invalid character - if(base64[src[0]] == 0x80) + if(base64[src[0]] == 0x80) break; // Check invalid character - if(base64[src[1]] == 0x80) + if(base64[src[1]] == 0x80) break; // Decode first byte @@ -994,7 +994,7 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) if(src[2] != '=') { // Check invalid character - if(base64[src[2]] == 0x80) + if(base64[src[2]] == 0x80) break; // Decode second byte @@ -1003,7 +1003,7 @@ ByteVector ByteVector::fromBase64(const ByteVector & input) if(src[3] != '=') { // Check invalid character - if(base64[src[3]] == 0x80) + if(base64[src[3]] == 0x80) break; // Decode third byte @@ -1059,7 +1059,7 @@ ByteVector ByteVector::toBase64() const *dst++ = alphabet[(src[0] & 0x03) << 4]; *dst++ = '='; } - *dst++='='; + *dst++ = '='; } return output; } From 2184aa476f19533d2829a40c7a7d22a76c8f8545 Mon Sep 17 00:00:00 2001 From: Sander Jansen Date: Mon, 18 May 2015 08:24:02 -0500 Subject: [PATCH 15/15] xiph: preserve any picture data we can't decode in the text comments --- taglib/ogg/xiphcomment.cpp | 101 +++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 39 deletions(-) diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 0424ed55..8ec108dd 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -419,51 +419,74 @@ void Ogg::XiphComment::parse(const ByteVector &data) // Handle Pictures separately if(entry.startsWith("METADATA_BLOCK_PICTURE=")) { - // Decode base64 picture data - ByteVector picturedata = ByteVector::fromBase64(entry.mid(23)); + // We need base64 encoded data including padding + if((entry.size() - 23) > 3 && ((entry.size() - 23) % 4) == 0) { - if(picturedata.size() == 0) { - debug("Empty picture data. Discarding content"); - continue; + // Decode base64 picture data + ByteVector picturedata = ByteVector::fromBase64(entry.mid(23)); + if (picturedata.size()) { + + // Decode Flac Picture + FLAC::Picture * picture = new FLAC::Picture(); + if(picture->parse(picturedata)) { + + d->pictureList.append(picture); + + // continue to next field + continue; + } + else { + delete picture; + debug("Failed to decode FlacPicture block"); + } + } + else { + debug("Failed to decode base64 encoded data"); + } } - - FLAC::Picture * picture = new FLAC::Picture(); - - if(picture->parse(picturedata)) - d->pictureList.append(picture); - else - debug("Unable to parse METADATA_BLOCK_PICTURE. Discarding content."); - } - else if (entry.startsWith("COVERART=")) { - - // Decode base64 picture data - ByteVector picturedata = ByteVector::fromBase64(entry.mid(9)); - - if (picturedata.size() == 0) { - debug("Empty coverart data. Discaring content"); - continue; + else { + debug("Invalid base64 encoded data"); } - - // Assume it's some type of image file - FLAC::Picture * picture = new FLAC::Picture(); - picture->setData(picturedata); - picture->setMimeType("image/"); - picture->setType(FLAC::Picture::Other); - d->pictureList.append(picture); } - else { - // Check for field separator - int sep = entry.find('='); - if (sep < 1) { - debug("Discarding invalid comment field."); - continue; + // Handle old picture standard + if (entry.startsWith("COVERART=")) { + + if((entry.size() - 9) > 3 && ((entry.size() - 9) % 4) == 0) { + + // Decode base64 picture data + ByteVector picturedata = ByteVector::fromBase64(entry.mid(9)); + if (picturedata.size()) { + + // Assume it's some type of image file + FLAC::Picture * picture = new FLAC::Picture(); + picture->setData(picturedata); + picture->setMimeType("image/"); + picture->setType(FLAC::Picture::Other); + d->pictureList.append(picture); + + // continue to next field + continue; + } + else { + debug("Failed to decode base64 encoded data"); + } + } + else { + debug("Invalid base64 encoded data"); } - - // Parse key and value - String key = String(entry.mid(0, sep), String::UTF8); - String value = String(entry.mid(sep + 1), String::UTF8); - addField(key, value, false); } + + // Check for field separator + int sep = entry.find('='); + if (sep < 1) { + debug("Discarding invalid comment field."); + continue; + } + + // Parse key and value + String key = String(entry.mid(0, sep), String::UTF8); + String value = String(entry.mid(sep + 1), String::UTF8); + addField(key, value, false); } }