Changeset 86425 in webkit


Ignore:
Timestamp:
May 13, 2011 6:29:58 AM (13 years ago)
Author:
hans@chromium.org
Message:

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):
Location:
trunk/Source
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r86424 r86425  
     12011-05-11  Hans Wennborg  <hans@chromium.org>
     2
     3        Reviewed by Tony Gentilcore.
     4
     5        IndexedDB: Fix integer comparison bug in LevelDB coding routines
     6        https://bugs.webkit.org/show_bug.cgi?id=60623
     7
     8        Fix the code for comparing two int64_t variables.
     9        Also remove faulty line in ObjectStoreNamesKey::encode which was
     10        uncovered by the unit test in this patch.
     11
     12        Very hard to cover with layout tests; covered by unit test.
     13
     14        * storage/IDBLevelDBCoding.cpp:
     15        (WebCore::IDBLevelDBCoding::compareInts):
     16        (WebCore::IDBLevelDBCoding::KeyPrefix::compare):
     17        (WebCore::IDBLevelDBCoding::DatabaseFreeListKey::compare):
     18        (WebCore::IDBLevelDBCoding::ObjectStoreMetaDataKey::compare):
     19        (WebCore::IDBLevelDBCoding::IndexMetaDataKey::compare):
     20        (WebCore::IDBLevelDBCoding::ObjectStoreFreeListKey::compare):
     21        (WebCore::IDBLevelDBCoding::IndexFreeListKey::compare):
     22        (WebCore::IDBLevelDBCoding::ObjectStoreNamesKey::encode):
     23        (WebCore::IDBLevelDBCoding::IndexNamesKey::compare):
     24        (WebCore::IDBLevelDBCoding::IndexDataKey::compare):
     25
    1262011-05-13  Andrew Wason  <rectalogic@rectalogic.com>
    227

  • trunk/Source/WebCore/storage/IDBLevelDBCoding.cpp

    r85844 r86425  
    198198
    199199    return ret;
     200}
     201
     202static int compareInts(int64_t a, int64_t b)
     203{
     204    ASSERT(a >= 0);
     205    ASSERT(b >= 0);
     206
     207    int64_t diff = a - b;
     208    if (diff < 0)
     209        return -1;
     210    if (diff > 0)
     211        return 1;
     212    return 0;
    200213}
    201214
    … …  
    661674
    662675    if (m_databaseId != other.m_databaseId)
    663         return m_databaseId - other.m_databaseId;
     676        return compareInts(m_databaseId, other.m_databaseId);
    664677    if (m_objectStoreId != other.m_objectStoreId)
    665         return m_objectStoreId - other.m_objectStoreId;
     678        return compareInts(m_objectStoreId, other.m_objectStoreId);
    666679    if (m_indexId != other.m_indexId)
    667         return m_indexId - other.m_indexId;
     680        return compareInts(m_indexId, other.m_indexId);
    668681    return 0;
    669682}
    … …  
    747760{
    748761    ASSERT(m_databaseId >= 0);
    749     return m_databaseId - other.m_databaseId;
     762    return compareInts(m_databaseId, other.m_databaseId);
    750763}
    751764
    … …  
    851864    ASSERT(m_objectStoreId >= 0);
    852865    ASSERT(m_metaDataType >= 0);
    853     if (int x = m_objectStoreId - other.m_objectStoreId)
    854         return x; // FIXME: Is this cast safe? I.e., will it preserve the sign?
     866    if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
     867        return x;
    855868    return m_metaDataType - other.m_metaDataType;
    856869}
    … …  
    906919    ASSERT(m_indexId >= 0);
    907920
    908     if (int x = m_objectStoreId - other.m_objectStoreId)
     921    if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
    909922        return x;
    910     if (int x = m_indexId - other.m_indexId)
     923    if (int x = compareInts(m_indexId, other.m_indexId))
    911924        return x;
    912925    return m_metaDataType - other.m_metaDataType;
    … …  
    963976    // We should probably make this more clear, though...
    964977    ASSERT(m_objectStoreId >= 0);
    965     return m_objectStoreId - other.m_objectStoreId;
     978    return compareInts(m_objectStoreId, other.m_objectStoreId);
    966979}
    967980
    … …  
    10071020    ASSERT(m_objectStoreId >= 0);
    10081021    ASSERT(m_indexId >= 0);
    1009     if (int x = m_objectStoreId - other.m_objectStoreId)
     1022    if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
    10101023        return x;
    1011     return m_indexId - other.m_indexId;
     1024    return compareInts(m_indexId, other.m_indexId);
    10121025}
    10131026
    … …  
    10471060    KeyPrefix prefix(databaseId, 0, 0);
    10481061    Vector<char> ret = prefix.encode();
    1049     ret.append(encodeByte(kSchemaVersionTypeByte));
    10501062    ret.append(encodeByte(kObjectStoreNamesTypeByte));
    10511063    ret.append(encodeStringWithLength(objectStoreName));
    … …  
    10991111{
    11001112    ASSERT(m_objectStoreId >= 0);
    1101     if (int x = m_objectStoreId - other.m_objectStoreId)
     1113    if (int x = compareInts(m_objectStoreId, other.m_objectStoreId))
    11021114        return x;
    11031115    return codePointCompare(m_indexName, other.m_indexName);
    … …  
    12451257    if (ignoreSequenceNumber)
    12461258        return 0;
    1247     return m_sequenceNumber - other.m_sequenceNumber;
     1259    return compareInts(m_sequenceNumber, other.m_sequenceNumber);
    12481260}
    12491261

  • trunk/Source/WebKit/chromium/ChangeLog

    r86422 r86425  
     12011-05-11  Hans Wennborg  <hans@chromium.org>
     2
     3        Reviewed by Tony Gentilcore.
     4
     5        IndexedDB: Fix integer comparison bug in LevelDB coding routines
     6        https://bugs.webkit.org/show_bug.cgi?id=60623
     7
     8        Unit test for comparison of encoded keys.
     9
     10        * tests/IDBLevelDBCodingTest.cpp:
     11        (IDBLevelDBCoding::TEST):
     12
    1132011-05-05  Hans Wennborg  <hans@chromium.org>
    214

  • trunk/Source/WebKit/chromium/tests/IDBLevelDBCodingTest.cpp

    r86330 r86425  
    3131
    3232#include "IDBKey.h"
     33#include "LevelDBSlice.h"
    3334#include <gtest/gtest.h>
    3435#include <wtf/Vector.h>
     36
     37#ifndef INT64_MAX
     38// FIXME: We shouldn't need to rely on these macros.
     39#define INT64_MAX 0x7fffffffffffffffLL
     40#endif
    3541
    3642using namespace WebCore;
    … …  
    205211    const int kLongStringLen = 1234;
    206212    UChar longString[kLongStringLen + 1];
    207     for (int i = 0; i < kLongStringLen; i++)
     213    for (int i = 0; i < kLongStringLen; ++i)
    208214        longString[i] = i;
    209215    longString[kLongStringLen] = 0;
    … …  
    303309    keys.append(IDBKey::createString("baaa"));
    304310
    305     for (size_t i = 0; i < keys.size() - 1; i++) {
     311    for (size_t i = 0; i < keys.size() - 1; ++i) {
    306312        RefPtr<IDBKey> keyA = keys[i];
    307313        RefPtr<IDBKey> keyB = keys[i + 1];
    … …  
    334340}
    335341
     342TEST(IDBLevelDBCodingTest, ComparisonTest)
     343{
     344    Vector<Vector<char> > keys;
     345    keys.append(SchemaVersionKey::encode());
     346    keys.append(MaxDatabaseIdKey::encode());
     347    keys.append(DatabaseFreeListKey::encode(0));
     348    keys.append(DatabaseFreeListKey::encode(INT64_MAX));
     349    keys.append(DatabaseNameKey::encode("", ""));
     350    keys.append(DatabaseNameKey::encode("", "a"));
     351    keys.append(DatabaseNameKey::encode("a", "a"));
     352    keys.append(DatabaseMetaDataKey::encode(1, DatabaseMetaDataKey::kOriginName));
     353    keys.append(ObjectStoreMetaDataKey::encode(1, 1, 0));
     354    keys.append(ObjectStoreMetaDataKey::encode(1, INT64_MAX, 0));
     355    keys.append(IndexMetaDataKey::encode(1, 1, 30, 0));
     356    keys.append(IndexMetaDataKey::encode(1, 1, INT64_MAX, 0));
     357    keys.append(ObjectStoreFreeListKey::encode(1, 1));
     358    keys.append(ObjectStoreFreeListKey::encode(1, INT64_MAX));
     359    keys.append(IndexFreeListKey::encode(1, 1, 30));
     360    keys.append(IndexFreeListKey::encode(1, 1, INT64_MAX));
     361    keys.append(IndexFreeListKey::encode(1, INT64_MAX, 30));
     362    keys.append(IndexFreeListKey::encode(1, INT64_MAX, INT64_MAX));
     363    keys.append(ObjectStoreNamesKey::encode(1, ""));
     364    keys.append(ObjectStoreNamesKey::encode(1, "a"));
     365    keys.append(IndexNamesKey::encode(1, 1, ""));
     366    keys.append(IndexNamesKey::encode(1, 1, "a"));
     367    keys.append(IndexNamesKey::encode(1, INT64_MAX, "a"));
     368    keys.append(ObjectStoreDataKey::encode(1, 1, minIDBKey()));
     369    keys.append(ObjectStoreDataKey::encode(1, 1, maxIDBKey()));
     370    keys.append(ExistsEntryKey::encode(1, 1, minIDBKey()));
     371    keys.append(ExistsEntryKey::encode(1, 1, maxIDBKey()));
     372    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), 0));
     373    keys.append(IndexDataKey::encode(1, 1, 30, minIDBKey(), INT64_MAX));
     374    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), 0));
     375    keys.append(IndexDataKey::encode(1, 1, 30, maxIDBKey(), INT64_MAX));
     376    keys.append(IndexDataKey::encode(1, 1, 31, minIDBKey(), 0));
     377    keys.append(IndexDataKey::encode(1, 2, 30, minIDBKey(), 0));
     378    keys.append(IndexDataKey::encodeMaxKey(1, 2));
     379    keys.append(ObjectStoreDataKey::encode(1, INT64_MAX, minIDBKey()));
     380    keys.append(ExistsEntryKey::encode(1, INT64_MAX, maxIDBKey()));
     381    keys.append(IndexDataKey::encodeMaxKey(1, INT64_MAX));
     382    keys.append(DatabaseMetaDataKey::encode(INT64_MAX, DatabaseMetaDataKey::kOriginName));
     383    keys.append(IndexDataKey::encodeMaxKey(INT64_MAX, INT64_MAX));
     384
     385    for (size_t i = 0; i < keys.size(); ++i) {
     386        const LevelDBSlice keyA(keys[i]);
     387        EXPECT_EQ(compare(keyA, keyA), 0);
     388
     389        for (size_t j = i + 1; j < keys.size(); ++j) {
     390            const LevelDBSlice keyB(keys[j]);
     391            EXPECT_LT(compare(keyA, keyB), 0);
     392            EXPECT_GT(compare(keyB, keyA), 0);
     393        }
     394    }
     395}
     396
    336397} // namespace
    337398
Note: See TracChangeset for help on using the changeset viewer.