Commit 4d38a498 authored by hans@chromium.org's avatar hans@chromium.org

2011-05-11 Hans Wennborg <hans@chromium.org>

        Reviewed by Tony Gentilcore.

        IndexedDB: Fix integer comparison bug in LevelDB coding routines
        https://bugs.webkit.org/show_bug.cgi?id=60623

        Fix the code for comparing two int64_t variables.
        Also remove faulty line in ObjectStoreNamesKey::encode which was
        uncovered by the unit test in this patch.

        Very hard to cover with layout tests; covered by unit test.

        * storage/IDBLevelDBCoding.cpp:
        (WebCore::IDBLevelDBCoding::compareInts):
        (WebCore::IDBLevelDBCoding::KeyPrefix::compare):
        (WebCore::IDBLevelDBCoding::DatabaseFreeListKey::compare):
        (WebCore::IDBLevelDBCoding::ObjectStoreMetaDataKey::compare):
        (WebCore::IDBLevelDBCoding::IndexMetaDataKey::compare):
        (WebCore::IDBLevelDBCoding::ObjectStoreFreeListKey::compare):
        (WebCore::IDBLevelDBCoding::IndexFreeListKey::compare):
        (WebCore::IDBLevelDBCoding::ObjectStoreNamesKey::encode):
        (WebCore::IDBLevelDBCoding::IndexNamesKey::compare):
        (WebCore::IDBLevelDBCoding::IndexDataKey::compare):
2011-05-11  Hans Wennborg  <hans@chromium.org>

        Reviewed by Tony Gentilcore.

        IndexedDB: Fix integer comparison bug in LevelDB coding routines
        https://bugs.webkit.org/show_bug.cgi?id=60623

        Unit test for comparison of encoded keys.

        * tests/IDBLevelDBCodingTest.cpp:
        (IDBLevelDBCoding::TEST):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@86425 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 02bc8e08
2011-05-11 Hans Wennborg <hans@chromium.org>
Reviewed by Tony Gentilcore.
IndexedDB: Fix integer comparison bug in LevelDB coding routines
https://bugs.webkit.org/show_bug.cgi?id=60623
Fix the code for comparing two int64_t variables.
Also remove faulty line in ObjectStoreNamesKey::encode which was
uncovered by the unit test in this patch.
Very hard to cover with layout tests; covered by unit test.
* storage/IDBLevelDBCoding.cpp:
(WebCore::IDBLevelDBCoding::compareInts):
(WebCore::IDBLevelDBCoding::KeyPrefix::compare):
(WebCore::IDBLevelDBCoding::DatabaseFreeListKey::compare):
(WebCore::IDBLevelDBCoding::ObjectStoreMetaDataKey::compare):
(WebCore::IDBLevelDBCoding::IndexMetaDataKey::compare):
(WebCore::IDBLevelDBCoding::ObjectStoreFreeListKey::compare):
(WebCore::IDBLevelDBCoding::IndexFreeListKey::compare):
(WebCore::IDBLevelDBCoding::ObjectStoreNamesKey::encode):
(WebCore::IDBLevelDBCoding::IndexNamesKey::compare):
(WebCore::IDBLevelDBCoding::IndexDataKey::compare):
2011-05-13 Andrew Wason <rectalogic@rectalogic.com>
Reviewed by Darin Adler.
......@@ -199,6 +199,19 @@ int64_t decodeInt(const char* begin, const char* end)
return ret;
}
static int compareInts(int64_t a, int64_t b)
{
ASSERT(a >= 0);
ASSERT(b >= 0);
int64_t diff = a - b;
if (diff < 0)
return -1;
if (diff > 0)
return 1;
return 0;
}
Vector<char> encodeVarInt(int64_t n)
{
Vector<char> ret; // FIXME: Size this at creation.
......@@ -660,11 +673,11 @@ int KeyPrefix::compare(const KeyPrefix& other) const
ASSERT(m_indexId != kInvalidId);
if (m_databaseId != other.m_databaseId)
return m_databaseId - other.m_databaseId;
return compareInts(m_databaseId, other.m_databaseId);
if (m_objectStoreId != other.m_objectStoreId)
return m_objectStoreId - other.m_objectStoreId;
return compareInts(m_objectStoreId, other.m_objectStoreId);
if (m_indexId != other.m_indexId)
return m_indexId - other.m_indexId;
return compareInts(m_indexId, other.m_indexId);
return 0;
}
......@@ -746,7 +759,7 @@ int64_t DatabaseFreeListKey::databaseId() const
int DatabaseFreeListKey::compare(const DatabaseFreeListKey& other) const
{
ASSERT(m_databaseId >= 0);
return m_databaseId - other.m_databaseId;
return compareInts(m_databaseId, other.m_databaseId);
}
const char* DatabaseNameKey::decode(const char* start, const char* limit, DatabaseNameKey* result)
......@@ -850,8 +863,8 @@ int ObjectStoreMetaDataKey::compare(const ObjectStoreMetaDataKey& other)
{
ASSERT(m_objectStoreId >= 0);
ASSERT(m_metaDataType >= 0);
if (int x = m_objectStoreId - other.m_objectStoreId)
return x; // FIXME: Is this cast safe? I.e., will it preserve the sign?
if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
return x;
return m_metaDataType - other.m_metaDataType;
}
......@@ -905,9 +918,9 @@ int IndexMetaDataKey::compare(const IndexMetaDataKey& other)
ASSERT(m_objectStoreId >= 0);
ASSERT(m_indexId >= 0);
if (int x = m_objectStoreId - other.m_objectStoreId)
if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
return x;
if (int x = m_indexId - other.m_indexId)
if (int x = compareInts(m_indexId, other.m_indexId))
return x;
return m_metaDataType - other.m_metaDataType;
}
......@@ -962,7 +975,7 @@ int ObjectStoreFreeListKey::compare(const ObjectStoreFreeListKey& other)
// but that comparison will have been made earlier.
// We should probably make this more clear, though...
ASSERT(m_objectStoreId >= 0);
return m_objectStoreId - other.m_objectStoreId;
return compareInts(m_objectStoreId, other.m_objectStoreId);
}
IndexFreeListKey::IndexFreeListKey()
......@@ -1006,9 +1019,9 @@ int IndexFreeListKey::compare(const IndexFreeListKey& other)
{
ASSERT(m_objectStoreId >= 0);
ASSERT(m_indexId >= 0);
if (int x = m_objectStoreId - other.m_objectStoreId)
if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
return x;
return m_indexId - other.m_indexId;
return compareInts(m_indexId, other.m_indexId);
}
int64_t IndexFreeListKey::objectStoreId() const
......@@ -1046,7 +1059,6 @@ Vector<char> ObjectStoreNamesKey::encode(int64_t databaseId, const String& objec
{
KeyPrefix prefix(databaseId, 0, 0);
Vector<char> ret = prefix.encode();
ret.append(encodeByte(kSchemaVersionTypeByte));
ret.append(encodeByte(kObjectStoreNamesTypeByte));
ret.append(encodeStringWithLength(objectStoreName));
return ret;
......@@ -1098,7 +1110,7 @@ Vector<char> IndexNamesKey::encode(int64_t databaseId, int64_t objectStoreId, co
int IndexNamesKey::compare(const IndexNamesKey& other)
{
ASSERT(m_objectStoreId >= 0);
if (int x = m_objectStoreId - other.m_objectStoreId)
if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
return x;
return codePointCompare(m_indexName, other.m_indexName);
}
......@@ -1244,7 +1256,7 @@ int IndexDataKey::compare(const IndexDataKey& other, bool ignoreSequenceNumber)
return x;
if (ignoreSequenceNumber)
return 0;
return m_sequenceNumber - other.m_sequenceNumber;
return compareInts(m_sequenceNumber, other.m_sequenceNumber);
}
int64_t IndexDataKey::databaseId() const
......
2011-05-11 Hans Wennborg <hans@chromium.org>
Reviewed by Tony Gentilcore.
IndexedDB: Fix integer comparison bug in LevelDB coding routines
https://bugs.webkit.org/show_bug.cgi?id=60623
Unit test for comparison of encoded keys.
* tests/IDBLevelDBCodingTest.cpp:
(IDBLevelDBCoding::TEST):
2011-05-05 Hans Wennborg <hans@chromium.org>
Reviewed by Steve Block.
......
......@@ -30,9 +30,15 @@
#if ENABLE(LEVELDB)
#include "IDBKey.h"
#include "LevelDBSlice.h"
#include <gtest/gtest.h>
#include <wtf/Vector.h>
#ifndef INT64_MAX
// FIXME: We shouldn't need to rely on these macros.
#define INT64_MAX 0x7fffffffffffffffLL
#endif
using namespace WebCore;
using namespace IDBLevelDBCoding;
......@@ -204,7 +210,7 @@ TEST(IDBLevelDBCodingTest, DecodeStringWithLength)
const int kLongStringLen = 1234;
UChar longString[kLongStringLen + 1];
for (int i = 0; i < kLongStringLen; i++)
for (int i = 0; i < kLongStringLen; ++i)
longString[i] = i;
longString[kLongStringLen] = 0;
......@@ -302,7 +308,7 @@ TEST(IDBLevelDBCodingTest, ExtractAndCompareIDBKeys)
keys.append(IDBKey::createString("b"));
keys.append(IDBKey::createString("baaa"));
for (size_t i = 0; i < keys.size() - 1; i++) {
for (size_t i = 0; i < keys.size() - 1; ++i) {
RefPtr<IDBKey> keyA = keys[i];
RefPtr<IDBKey> keyB = keys[i + 1];
......@@ -333,6 +339,61 @@ TEST(IDBLevelDBCodingTest, ExtractAndCompareIDBKeys)
}
}
TEST(IDBLevelDBCodingTest, ComparisonTest)
{
Vector<Vector<char> > keys;
keys.append(SchemaVersionKey::encode());
keys.append(MaxDatabaseIdKey::encode());
keys.append(DatabaseFreeListKey::encode(0));
keys.append(DatabaseFreeListKey::encode(INT64_MAX));
keys.append(DatabaseNameKey::encode("", ""));
keys.append(DatabaseNameKey::encode("", "a"));
keys.append(DatabaseNameKey::encode("a", "a"));
keys.append(DatabaseMetaDataKey::encode(1, DatabaseMetaDataKey::kOriginName));
keys.append(ObjectStoreMetaDataKey::encode(1, 1, 0));
keys.append(ObjectStoreMetaDataKey::encode(1, INT64_MAX, 0));
keys.append(IndexMetaDataKey::encode(1, 1, 30, 0));
keys.append(IndexMetaDataKey::encode(1, 1, INT64_MAX, 0));
keys.append(ObjectStoreFreeListKey::encode(1, 1));
keys.append(ObjectStoreFreeListKey::encode(1, INT64_MAX));
keys.append(IndexFreeListKey::encode(1, 1, 30));
keys.append(IndexFreeListKey::encode(1, 1, INT64_MAX));
keys.append(IndexFreeListKey::encode(1, INT64_MAX, 30));
keys.append(IndexFreeListKey::encode(1, INT64_MAX, INT64_MAX));
keys.append(ObjectStoreNamesKey::encode(1, ""));
keys.append(ObjectStoreNamesKey::encode(1, "a"));
keys.append(IndexNamesKey::encode(1, 1, ""));
keys.append(IndexNamesKey::encode(1, 1, "a"));
keys.append(IndexNamesKey::encode(1, INT64_MAX, "a"));
keys.append(ObjectStoreDataKey::encode(1, 1, minIDBKey()));
keys.append(ObjectStoreDataKey::encode(1, 1, maxIDBKey()));
keys.append(ExistsEntryKey::encode(1, 1, minIDBKey()));
keys.append(ExistsEntryKey::encode(1, 1, maxIDBKey()));
keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), 0));
keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), INT64_MAX));
keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), 0));
keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), INT64_MAX));
keys.append(IndexDataKey::encode(1, 1, 31, minIDBKey(), 0));
keys.append(IndexDataKey::encode(1, 2, 30, minIDBKey(), 0));
keys.append(IndexDataKey::encodeMaxKey(1, 2));
keys.append(ObjectStoreDataKey::encode(1, INT64_MAX, minIDBKey()));
keys.append(ExistsEntryKey::encode(1, INT64_MAX, maxIDBKey()));
keys.append(IndexDataKey::encodeMaxKey(1, INT64_MAX));
keys.append(DatabaseMetaDataKey::encode(INT64_MAX, DatabaseMetaDataKey::kOriginName));
keys.append(IndexDataKey::encodeMaxKey(INT64_MAX, INT64_MAX));
for (size_t i = 0; i < keys.size(); ++i) {
const LevelDBSlice keyA(keys[i]);
EXPECT_EQ(compare(keyA, keyA), 0);
for (size_t j = i + 1; j < keys.size(); ++j) {
const LevelDBSlice keyB(keys[j]);
EXPECT_LT(compare(keyA, keyB), 0);
EXPECT_GT(compare(keyB, keyA), 0);
}
}
}
} // namespace
#endif // ENABLE(LEVELDB)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment