Merge remote-tracking branch 'supermihi/master'

This commit is contained in:
Lukáš Lalinský 2012-08-23 10:17:05 +02:00
commit 29d17bb8e9
15 changed files with 159 additions and 72 deletions

View File

@ -189,7 +189,7 @@ PropertyMap APE::Tag::properties() const
PropertyMap properties;
ItemListMap::ConstIterator it = itemListMap().begin();
for(; it != itemListMap().end(); ++it) {
String tagName = PropertyMap::prepareKey(it->first);
String tagName = it->first.upper();
// if the item is Binary or Locator, or if the key is an invalid string,
// add to unsupportedData
if(it->second.type() != Item::Text || tagName.isNull())
@ -227,7 +227,7 @@ PropertyMap APE::Tag::setProperties(const PropertyMap &origProps)
StringList toRemove;
ItemListMap::ConstIterator remIt = itemListMap().begin();
for(; remIt != itemListMap().end(); ++remIt) {
String key = PropertyMap::prepareKey(remIt->first);
String key = remIt->first.upper();
// only remove if a) key is valid, b) type is text, c) key not contained in new properties
if(!key.isNull() && remIt->second.type() == APE::Item::Text && !properties.contains(key))
toRemove.append(remIt->first);
@ -238,9 +238,12 @@ PropertyMap APE::Tag::setProperties(const PropertyMap &origProps)
// now sync in the "forward direction"
PropertyMap::ConstIterator it = properties.begin();
PropertyMap invalid;
for(; it != properties.end(); ++it) {
const String &tagName = it->first;
if(!(itemListMap().contains(tagName)) || !(itemListMap()[tagName].values() == it->second)) {
if(!checkKey(tagName))
invalid.insert(it->first, it->second);
else if(!(itemListMap().contains(tagName)) || !(itemListMap()[tagName].values() == it->second)) {
if(it->second.size() == 0)
removeItem(tagName);
else {
@ -252,7 +255,21 @@ PropertyMap APE::Tag::setProperties(const PropertyMap &origProps)
}
}
}
return PropertyMap();
return invalid;
}
bool APE::Tag::checkKey(const String &key)
{
if(key.size() < 2 or key.size() > 16)
return false;
for(String::ConstIterator it = key.begin(); it != key.end(); it++)
// only allow printable ASCII including space (32..127)
if (*it < 32 || *it >= 128)
return false;
String upperKey = key.upper();
if (upperKey=="ID3" || upperKey=="TAG" || upperKey=="OGGS" || upperKey=="MP+")
return false;
return true;
}
APE::Footer *APE::Tag::footer() const

View File

@ -123,10 +123,17 @@ namespace TagLib {
/*!
* Implements the unified tag dictionary interface -- import function. The same
* comments as for the export function apply.
* comments as for the export function apply; additionally note that the APE tag
* specification requires keys to have between 2 and 16 printable ASCII characters
* with the exception of the fixed strings "ID3", "TAG", "OGGS", and "MP+".
*/
PropertyMap setProperties(const PropertyMap &);
/*!
* Check if the given String is a valid APE tag key.
*/
static bool checkKey(const String&);
/*!
* Returns a pointer to the tag's footer.
*/

View File

@ -112,7 +112,7 @@ void CommentsFrame::setTextEncoding(String::Type encoding)
PropertyMap CommentsFrame::asProperties() const
{
String key = PropertyMap::prepareKey(description());
String key = description().upper();
PropertyMap map;
if(key.isEmpty() || key == "COMMENT")
map.insert("COMMENT", text());

View File

@ -289,7 +289,7 @@ PropertyMap TextIdentificationFrame::makeTMCLProperties() const
}
StringList l = fieldList();
for(StringList::ConstIterator it = l.begin(); it != l.end(); ++it) {
String instrument = PropertyMap::prepareKey(*it);
String instrument = it->upper();
if(instrument.isNull()) {
// instrument is not a valid key -> frame unsupported
map.clear();
@ -382,7 +382,7 @@ PropertyMap UserTextIdentificationFrame::asProperties() const
String tagName = description();
PropertyMap map;
String key = map.prepareKey(tagName);
String key = tagName.upper();
if(key.isNull()) // this frame's description is not a valid PropertyMap key -> add to unsupported list
map.unsupportedData().append(L"TXXX/" + description());
else {

View File

@ -116,7 +116,7 @@ void UnsynchronizedLyricsFrame::setTextEncoding(String::Type encoding)
PropertyMap UnsynchronizedLyricsFrame::asProperties() const
{
PropertyMap map;
String key = PropertyMap::prepareKey(description());
String key = description().upper();
if(key.isEmpty() || key.upper() == "LYRICS")
map.insert("LYRICS", text());
else if(key.isNull())

View File

@ -156,7 +156,7 @@ void UserUrlLinkFrame::setDescription(const String &s)
PropertyMap UserUrlLinkFrame::asProperties() const
{
PropertyMap map;
String key = PropertyMap::prepareKey(description());
String key = description().upper();
if(key.isEmpty() || key.upper() == "URL")
map.insert("URL", url());
else if(key.isNull())

View File

@ -205,11 +205,14 @@ PropertyMap Ogg::XiphComment::setProperties(const PropertyMap &properties)
for(StringList::ConstIterator it = toRemove.begin(); it != toRemove.end(); ++it)
removeField(*it);
// now go through keys in \a properties and check that the values match those in the xiph comment */
// now go through keys in \a properties and check that the values match those in the xiph comment
PropertyMap invalid;
PropertyMap::ConstIterator it = properties.begin();
for(; it != properties.end(); ++it)
{
if(!d->fieldListMap.contains(it->first) || !(it->second == d->fieldListMap[it->first])) {
if(!checkKey(it->first))
invalid.insert(it->first, it->second);
else if(!d->fieldListMap.contains(it->first) || !(it->second == d->fieldListMap[it->first])) {
const StringList &sl = it->second;
if(sl.size() == 0)
// zero size string list -> remove the tag with all values
@ -224,7 +227,18 @@ PropertyMap Ogg::XiphComment::setProperties(const PropertyMap &properties)
}
}
}
return PropertyMap();
return invalid;
}
bool Ogg::XiphComment::checkKey(const String &key)
{
if(key.size() < 1)
return false;
for(String::ConstIterator it = key.begin(); it != key.end(); it++)
// forbid non-printable, non-ascii, '=' (#61) and '~' (#126)
if (*it < 32 || *it >= 128 || *it == 61 || *it == 126)
return false;
return true;
}
String Ogg::XiphComment::vendorID() const

View File

@ -151,10 +151,18 @@ namespace TagLib {
/*!
* Implements the unified property interface -- import function.
* The tags from the given map will be stored one-to-one in the file.
* The tags from the given map will be stored one-to-one in the file,
* except for invalid keys (less than one character, non-ASCII, or
* containing '=' or '~') in which case the according values will
* be contained in the returned PropertyMap.
*/
PropertyMap setProperties(const PropertyMap&);
/*!
* Check if the given String is a valid Xiph comment key.
*/
static bool checkKey(const String&);
/*!
* Returns the vendor ID of the Ogg Vorbis encoder. libvorbis 1.0 as the
* most common case always returns "Xiph.Org libVorbis I 20020717".

View File

@ -34,7 +34,7 @@ PropertyMap::PropertyMap(const PropertyMap &m) : SimplePropertyMap(m), unsupport
PropertyMap::PropertyMap(const SimplePropertyMap &m)
{
for(SimplePropertyMap::ConstIterator it = m.begin(); it != m.end(); ++it){
String key = prepareKey(it->first);
String key = it->first.upper();
if(!key.isNull())
insert(it->first, it->second);
else
@ -48,10 +48,7 @@ PropertyMap::~PropertyMap()
bool PropertyMap::insert(const String &key, const StringList &values)
{
String realKey = prepareKey(key);
if(realKey.isNull())
return false;
String realKey = key.upper();
Iterator result = SimplePropertyMap::find(realKey);
if(result == end())
SimplePropertyMap::insert(realKey, values);
@ -62,9 +59,7 @@ bool PropertyMap::insert(const String &key, const StringList &values)
bool PropertyMap::replace(const String &key, const StringList &values)
{
String realKey = prepareKey(key);
if(realKey.isNull())
return false;
String realKey = key.upper();
SimplePropertyMap::erase(realKey);
SimplePropertyMap::insert(realKey, values);
return true;
@ -72,26 +67,17 @@ bool PropertyMap::replace(const String &key, const StringList &values)
PropertyMap::Iterator PropertyMap::find(const String &key)
{
String realKey = prepareKey(key);
if(realKey.isNull())
return end();
return SimplePropertyMap::find(realKey);
return SimplePropertyMap::find(key.upper());
}
PropertyMap::ConstIterator PropertyMap::find(const String &key) const
{
String realKey = prepareKey(key);
if(realKey.isNull())
return end();
return SimplePropertyMap::find(realKey);
return SimplePropertyMap::find(key.upper());
}
bool PropertyMap::contains(const String &key) const
{
String realKey = prepareKey(key);
if(realKey.isNull())
return false;
return SimplePropertyMap::contains(realKey);
return SimplePropertyMap::contains(key.upper());
}
bool PropertyMap::contains(const PropertyMap &other) const
@ -107,9 +93,7 @@ bool PropertyMap::contains(const PropertyMap &other) const
PropertyMap &PropertyMap::erase(const String &key)
{
String realKey = prepareKey(key);
if(!realKey.isNull())
SimplePropertyMap::erase(realKey);
SimplePropertyMap::erase(key.upper());
return *this;
}
@ -122,23 +106,20 @@ PropertyMap &PropertyMap::erase(const PropertyMap &other)
PropertyMap &PropertyMap::merge(const PropertyMap &other)
{
for(PropertyMap::ConstIterator it = other.begin(); it != other.end(); ++it) {
for(PropertyMap::ConstIterator it = other.begin(); it != other.end(); ++it)
insert(it->first, it->second);
}
unsupported.append(other.unsupported);
return *this;
}
const StringList &PropertyMap::operator[](const String &key) const
{
String realKey = prepareKey(key);
return SimplePropertyMap::operator[](realKey);
return SimplePropertyMap::operator[](key.upper());
}
StringList &PropertyMap::operator[](const String &key)
{
String realKey = prepareKey(key);
return SimplePropertyMap::operator[](realKey);
return SimplePropertyMap::operator[](key.upper());
}
bool PropertyMap::operator==(const PropertyMap &other) const
@ -190,13 +171,3 @@ const StringList &PropertyMap::unsupportedData() const
{
return unsupported;
}
String PropertyMap::prepareKey(const String &proposed) {
if(proposed.isEmpty())
return String::null;
for (String::ConstIterator it = proposed.begin(); it != proposed.end(); it++)
// forbid non-printable, non-ascii, '=' (#61) and '~' (#126)
if (*it < 32 || *it >= 128 || *it == 61 || *it == 126)
return String::null;
return proposed.upper();
}

View File

@ -36,15 +36,10 @@ namespace TagLib {
* ("tags") realized as pairs of a case-insensitive key
* and a nonempty list of corresponding values, each value being an an arbitrary
* unicode String.
* The key has the same restrictions as in the vorbis comment specification,
* i.e. it must contain at least one character; all printable ASCII characters
* except '=' and '~' are allowed.
*
* In order to be safe with other formats, keep these additional restrictions in mind:
*
* - APE only allows keys from 2 to 16 printable ASCII characters (including space),
* with the exception of these strings: ID3, TAG, OggS, MP+
*
* Note that most metadata formats pose additional conditions on the tag keys. The
* most popular ones (Vorbis, APE, ID3v2) should support all ASCII only words of
* length between 2 and 16.
*/
class TAGLIB_EXPORT PropertyMap: public SimplePropertyMap
@ -126,16 +121,17 @@ namespace TagLib {
/*!
* Returns a reference to the value associated with \a key.
*
* \note: This has undefined behavior if the key is not valid or not
* present in the map.
* \note: If \a key is not contained in the map, an empty
* StringList is returned without error.
*/
const StringList &operator[](const String &key) const;
/*!
* Returns a reference to the value associated with \a key.
*
* \note: This has undefined behavior if the key is not valid or not
* present in the map.
* \note: If \a key is not contained in the map, an empty
* StringList is returned. You can also directly add entries
* by using this function as an lvalue.
*/
StringList &operator[](const String &key);
@ -168,12 +164,6 @@ namespace TagLib {
String toString() const;
/*!
* Converts \a proposed into another String suitable to be used as
* a key, or returns String::null if this is not possible.
*/
static String prepareKey(const String &proposed);
private:

View File

@ -35,6 +35,7 @@ SET(test_runner_SRCS
test_bytevectorlist.cpp
test_bytevectorstream.cpp
test_string.cpp
test_propertymap.cpp
test_fileref.cpp
test_id3v1.cpp
test_id3v2.cpp

View File

@ -19,6 +19,7 @@ class TestAPETag : public CppUnit::TestFixture
CPPUNIT_TEST(testIsEmpty2);
CPPUNIT_TEST(testPropertyInterface1);
CPPUNIT_TEST(testPropertyInterface2);
CPPUNIT_TEST(testInvalidKeys);
CPPUNIT_TEST_SUITE_END();
public:
@ -76,12 +77,27 @@ public:
APE::Item item3 = APE::Item("TRACKNUMBER", "29");
tag.setItem("TRACKNUMBER", item3);
properties = tag.properties();
CPPUNIT_ASSERT_EQUAL(2u, properties["TRACKNUMBER"].size());
CPPUNIT_ASSERT_EQUAL(uint(2), properties["TRACKNUMBER"].size());
CPPUNIT_ASSERT_EQUAL(String("17"), properties["TRACKNUMBER"][0]);
CPPUNIT_ASSERT_EQUAL(String("29"), properties["TRACKNUMBER"][1]);
}
void testInvalidKeys()
{
PropertyMap properties;
properties["A"] = String("invalid key: one character");
properties["MP+"] = String("invalid key: forbidden string");
properties["A B~C"] = String("valid key: space and tilde");
properties["ARTIST"] = String("valid key: normal one");
APE::Tag tag;
PropertyMap unsuccessful = tag.setProperties(properties);
CPPUNIT_ASSERT_EQUAL(uint(2), unsuccessful.size());
CPPUNIT_ASSERT(unsuccessful.contains("A"));
CPPUNIT_ASSERT(unsuccessful.contains("MP+"));
}
};
CPPUNIT_TEST_SUITE_REGISTRATION(TestAPETag);

View File

@ -72,6 +72,7 @@ class TestID3v2 : public CppUnit::TestFixture
CPPUNIT_TEST(testW000);
CPPUNIT_TEST(testPropertyInterface);
CPPUNIT_TEST(testPropertyInterface2);
CPPUNIT_TEST(testDeleteFrame);
CPPUNIT_TEST(testSaveAndStripID3v1ShouldNotAddFrameFromID3v1ToId3v2);
CPPUNIT_TEST_SUITE_END();
@ -640,6 +641,21 @@ public:
CPPUNIT_ASSERT(tag.frameList("TIPL").isEmpty());
}
void testDeleteFrame()
{
ScopedFileCopy copy("rare_frames", ".mp3");
string newname = copy.fileName();
MPEG::File f(newname.c_str());
ID3v2::Tag *t = f.ID3v2Tag();
ID3v2::Frame *frame = t->frameList("TCON")[0];
CPPUNIT_ASSERT_EQUAL(1u, t->frameList("TCON").size());
t->removeFrame(frame, true);
f.save(MPEG::File::ID3v2);
MPEG::File f2(newname.c_str());
t = f2.ID3v2Tag();
CPPUNIT_ASSERT(t->frameList("TCON").isEmpty());
}
void testSaveAndStripID3v1ShouldNotAddFrameFromID3v1ToId3v2()
{
ScopedFileCopy copy("xing", ".mp3");

View File

@ -0,0 +1,32 @@
#include <cppunit/extensions/HelperMacros.h>
#include <tpropertymap.h>
class TestPropertyMap : public CppUnit::TestFixture
{
CPPUNIT_TEST_SUITE(TestPropertyMap);
CPPUNIT_TEST(testInvalidKeys);
CPPUNIT_TEST_SUITE_END();
public:
void testInvalidKeys()
{
TagLib::PropertyMap map1;
CPPUNIT_ASSERT(map1.isEmpty());
map1["ÄÖÜ"].append("test");
CPPUNIT_ASSERT_EQUAL(map1.size(), 1u);
TagLib::PropertyMap map2;
map2["ÄÖÜ"].append("test");
CPPUNIT_ASSERT(map1 == map2);
CPPUNIT_ASSERT(map1.contains(map2));
map2["ARTIST"] = TagLib::String("Test Artist");
CPPUNIT_ASSERT(map1 != map2);
CPPUNIT_ASSERT(map2.contains(map1));
map2["ÄÖÜ"].append("test 2");
CPPUNIT_ASSERT(!map2.contains(map1));
}
};
CPPUNIT_TEST_SUITE_REGISTRATION(TestPropertyMap);

View File

@ -2,6 +2,7 @@
#include <string>
#include <stdio.h>
#include <xiphcomment.h>
#include <tpropertymap.h>
#include <tdebug.h>
#include "utils.h"
@ -15,6 +16,7 @@ class TestXiphComment : public CppUnit::TestFixture
CPPUNIT_TEST(testSetYear);
CPPUNIT_TEST(testTrack);
CPPUNIT_TEST(testSetTrack);
CPPUNIT_TEST(testInvalidKeys);
CPPUNIT_TEST_SUITE_END();
public:
@ -59,6 +61,19 @@ public:
CPPUNIT_ASSERT_EQUAL(String("3"), cmt.fieldListMap()["TRACKNUMBER"].front());
}
void testInvalidKeys()
{
PropertyMap map;
map[""] = String("invalid key: empty string");
map["A=B"] = String("invalid key: contains '='");
map["A~B"] = String("invalid key: contains '~'");
Ogg::XiphComment cmt;
PropertyMap unsuccessful = cmt.setProperties(map);
CPPUNIT_ASSERT_EQUAL(TagLib::uint(3), unsuccessful.size());
CPPUNIT_ASSERT(cmt.properties().isEmpty());
}
};
CPPUNIT_TEST_SUITE_REGISTRATION(TestXiphComment);