From 5db1507ad5d8ed21ebed23c70bc4cdfa305bc959 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Wed, 28 Sep 2022 15:02:31 +0200 Subject: [PATCH 1/6] Fix count of mutable copy of dict with undefined value --- Fleece/Core/Dict.cc | 48 +++++++++++++++++++------------------- Fleece/Core/Dict.hh | 8 +++---- Fleece/Mutable/HeapDict.cc | 4 ++-- Tests/API_ValueTests.cc | 14 +++++++++++ 4 files changed, 44 insertions(+), 30 deletions(-) diff --git a/Fleece/Core/Dict.cc b/Fleece/Core/Dict.cc index 4490eabd..607823d2 100644 --- a/Fleece/Core/Dict.cc +++ b/Fleece/Core/Dict.cc @@ -65,25 +65,25 @@ namespace fleece { namespace impl { template __hot - const Value* finishGet(const Value *keyFound, KEY &keyToFind) const noexcept { + const Value* finishGet(const Value *keyFound, KEY &keyToFind, bool returnUndefined) const noexcept { if (keyFound) { auto value = deref(next(keyFound)); - if (_usuallyFalse(value->isUndefined())) + if (!returnUndefined && _usuallyFalse(value->isUndefined())) value = nullptr; return value; } else { const Dict *parent = getParent(); - return parent ? parent->get(keyToFind) : nullptr; + return parent ? parent->get(keyToFind, returnUndefined) : nullptr; } } __hot - inline const Value* getUnshared(slice keyToFind) const noexcept { + inline const Value* getUnshared(slice keyToFind, bool returnUndefined) const noexcept { auto key = search(keyToFind, [](slice target, const Value *val) { countComparison(); return compareKeys(target, val); }); - return finishGet(key, keyToFind); + return finishGet(key, keyToFind, returnUndefined); } __hot @@ -95,25 +95,25 @@ namespace fleece { namespace impl { } __hot - inline const Value* get(int keyToFind) const noexcept { + inline const Value* get(int keyToFind, bool returnUndefined= false) const noexcept { assert_precondition(keyToFind >= 0); - return finishGet(search(keyToFind), keyToFind); + return finishGet(search(keyToFind), keyToFind, returnUndefined); } __hot - inline const Value* get(slice keyToFind, SharedKeys *sharedKeys =nullptr) const noexcept { + inline const Value* get(slice keyToFind, SharedKeys *sharedKeys =nullptr, bool returnUndefined= false) const noexcept { if (!sharedKeys && usesSharedKeys()) { sharedKeys = findSharedKeys(); assert_precondition(sharedKeys || gDisableNecessarySharedKeysCheck); } int encoded; if (sharedKeys && lookupSharedKey(keyToFind, sharedKeys, encoded)) - return get(encoded); - return getUnshared(keyToFind); + return get(encoded, returnUndefined); + return getUnshared(keyToFind, returnUndefined); } __hot - const Value* get(Dict::key &keyToFind) const noexcept { + const Value* get(Dict::key &keyToFind, bool returnUndefined= false) const noexcept { auto sharedKeys = keyToFind._sharedKeys; if (!sharedKeys && usesSharedKeys()) { sharedKeys = findSharedKeys(); @@ -123,13 +123,13 @@ namespace fleece { namespace impl { if (_usuallyTrue(sharedKeys != nullptr)) { // Look for a numeric key first: if (_usuallyTrue(keyToFind._hasNumericKey)) - return get(keyToFind._numericKey); + return get(keyToFind._numericKey, returnUndefined); // Key was not registered last we checked; see if dict contains any new keys: if (_usuallyFalse(_count == 0)) return nullptr; if (lookupSharedKey(keyToFind._rawString, sharedKeys, keyToFind._numericKey)) { keyToFind._hasNumericKey = true; - return get(keyToFind._numericKey); + return get(keyToFind._numericKey, returnUndefined); } } @@ -137,7 +137,7 @@ namespace fleece { namespace impl { const Value *key = findKeyByHint(keyToFind); if (!key) key = findKeyBySearch(keyToFind); - return finishGet(key, keyToFind); + return finishGet(key, keyToFind, returnUndefined); } key_t encodeKey(slice keyString, SharedKeys *sharedKeys) const noexcept { @@ -327,26 +327,26 @@ namespace fleece { namespace impl { } __hot - const Value* Dict::get(slice keyToFind) const noexcept { + const Value* Dict::get(slice keyToFind, bool returnUndefined) const noexcept { if (_usuallyFalse(isMutable())) return heapDict()->get(keyToFind); if (isWideArray()) - return dictImpl(this).get(keyToFind); + return dictImpl(this).get(keyToFind, nullptr, returnUndefined); else - return dictImpl(this).get(keyToFind); + return dictImpl(this).get(keyToFind, nullptr, returnUndefined); } __hot - const Value* Dict::get(int keyToFind) const noexcept { + const Value* Dict::get(int keyToFind, bool returnUndefined) const noexcept { if (_usuallyFalse(isMutable())) return heapDict()->get(keyToFind); else if (isWideArray()) - return dictImpl(this).get(keyToFind); + return dictImpl(this).get(keyToFind, returnUndefined); else - return dictImpl(this).get(keyToFind); + return dictImpl(this).get(keyToFind, returnUndefined); } - const Value* Dict::get(key &keyToFind) const noexcept { + const Value* Dict::get(key &keyToFind, bool returnUndefined) const noexcept { if (_usuallyFalse(isMutable())) return heapDict()->get(keyToFind); else if (isWideArray()) @@ -356,13 +356,13 @@ namespace fleece { namespace impl { } __hot - const Value* Dict::get(const key_t &keyToFind) const noexcept { + const Value* Dict::get(const key_t &keyToFind, bool returnUndefined) const noexcept { if (_usuallyFalse(isMutable())) return heapDict()->get(keyToFind); else if (keyToFind.shared()) - return get(keyToFind.asInt()); + return get(keyToFind.asInt(), returnUndefined); else - return get(keyToFind.asString()); + return get(keyToFind.asString(), returnUndefined); } key_t Dict::encodeKey(slice keyString, SharedKeys *sharedKeys) const noexcept { diff --git a/Fleece/Core/Dict.hh b/Fleece/Core/Dict.hh index 211abbfe..6db8ebd4 100644 --- a/Fleece/Core/Dict.hh +++ b/Fleece/Core/Dict.hh @@ -30,10 +30,10 @@ namespace fleece { namespace impl { bool empty() const noexcept FLPURE; /** Looks up the Value for a string key. */ - const Value* get(slice keyToFind) const noexcept FLPURE; + const Value* get(slice keyToFind, bool returnUndefined= false) const noexcept FLPURE; /** Looks up the Value for an integer (shared) key. */ - const Value* get(int numericKeyToFind) const noexcept FLPURE; + const Value* get(int numericKeyToFind, bool returnUndefined= false) const noexcept FLPURE; /** If this array is mutable, returns the equivalent MutableArray*, else returns nullptr. */ MutableDict* asMutable() const noexcept FLPURE; @@ -84,9 +84,9 @@ namespace fleece { namespace impl { /** Looks up the Value for a key, in a form that can cache the key's Fleece object. Using the Fleece object is significantly faster than a normal get. */ - const Value* get(key&) const noexcept; + const Value* get(key&, bool returnUndefined= false) const noexcept; - const Value* get(const key_t&) const noexcept; + const Value* get(const key_t&, bool returnUndefined= false) const noexcept; constexpr Dict() :Value(internal::kDictTag, 0, 0) { } diff --git a/Fleece/Mutable/HeapDict.cc b/Fleece/Mutable/HeapDict.cc index e0cf4c07..80e4de74 100644 --- a/Fleece/Mutable/HeapDict.cc +++ b/Fleece/Mutable/HeapDict.cc @@ -115,7 +115,7 @@ namespace fleece { namespace impl { namespace internal { key = encodeKey(stringKey); slotp = &_makeValueFor(key); } - if (slotp->empty() && !(_source && _source->get(key))) + if (slotp->empty() && !(_source && _source->get(key, /*returnUndefined =*/ true))) ++_count; markChanged(); return *slotp; @@ -177,7 +177,7 @@ namespace fleece { namespace impl { namespace internal { void HeapDict::remove(slice stringKey) { key_t key = encodeKey(stringKey); - if (_source && _source->get(key)) { + if (_source && _source->get(key, /*returnUndefined =*/ true)) { auto it = _map.find(key); if (it != _map.end()) { if (_usuallyFalse(!it->second)) diff --git a/Tests/API_ValueTests.cc b/Tests/API_ValueTests.cc index 8047e9ba..c6310f30 100644 --- a/Tests/API_ValueTests.cc +++ b/Tests/API_ValueTests.cc @@ -366,3 +366,17 @@ TEST_CASE("API MutableDict item bool conversion", "[API]") { } CHECK(dict.toJSONString() == "{\"a_key\":6}"); } + + +TEST_CASE("API Mutable copy of Dict with undefined value", "[API]") { + auto enc = fleece::Encoder(); + enc.beginDict(); + enc.writeKey("a"); + enc.writeUndefined(); + enc.endDict(); + auto data = enc.finish(); + auto doc = Doc(data); + auto copy = doc.root().asDict().mutableCopy(kFLCopyImmutables); + CHECK(copy.count() == 1); + CHECK(copy["a"].type() == kFLUndefined); +} From 230896fbaadf6eb4545dccd7f7367f496bae8d46 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Wed, 28 Sep 2022 15:03:29 +0200 Subject: [PATCH 2/6] Fix assert to handle `nan` --- Fleece/Mutable/ValueSlot.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Fleece/Mutable/ValueSlot.cc b/Fleece/Mutable/ValueSlot.cc index a21d7fbd..591f5d22 100644 --- a/Fleece/Mutable/ValueSlot.cc +++ b/Fleece/Mutable/ValueSlot.cc @@ -16,6 +16,7 @@ #include "Encoder.hh" #include "varint.hh" #include +#include namespace fleece { namespace impl { using namespace std; @@ -176,7 +177,7 @@ namespace fleece { namespace impl { } else { setPointer(HeapValue::create(d)->asValue()); } - assert_postcondition(asValue()->asDouble() == d); + assert_postcondition(asValue()->asDouble() == d || (isnan(asValue()->asDouble()) && isnan(d))); } From ce6afd2420a4cbee7b5701a7cebae02013a1c400 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Wed, 28 Sep 2022 15:04:20 +0200 Subject: [PATCH 3/6] Expand validation of untrusted Fleece data --- Fleece/Core/Dict.cc | 2 +- Fleece/Core/Dict.hh | 5 +- Fleece/Core/Pointer.cc | 5 +- Fleece/Core/Pointer.hh | 3 +- Fleece/Core/SharedKeys.hh | 5 +- Fleece/Core/Value+Dump.cc | 2 +- Fleece/Core/Value.cc | 158 ++++++++++++++++++++++++++++++++++---- Fleece/Core/Value.hh | 8 +- Tests/SharedKeysTests.cc | 3 + 9 files changed, 169 insertions(+), 22 deletions(-) diff --git a/Fleece/Core/Dict.cc b/Fleece/Core/Dict.cc index 607823d2..9a64586b 100644 --- a/Fleece/Core/Dict.cc +++ b/Fleece/Core/Dict.cc @@ -278,7 +278,7 @@ namespace fleece { namespace impl { __hot - static int compareKeys(const Value *keyToFind, const Value *key, bool wide) { + int compareKeys(const Value *keyToFind, const Value *key, bool wide) { if (wide) return dictImpl::compareKeys(keyToFind, key); else diff --git a/Fleece/Core/Dict.hh b/Fleece/Core/Dict.hh index 6db8ebd4..ea4dbf77 100644 --- a/Fleece/Core/Dict.hh +++ b/Fleece/Core/Dict.hh @@ -21,6 +21,9 @@ namespace fleece { namespace impl { class SharedKeys; class key_t; + /** Compares Dict keys in the order in which they must appear in Fleece data. */ + int compareKeys(const Value *a, const Value *b, bool wide); + /** A Value that's a dictionary/map */ class Dict : public Value { public: @@ -72,7 +75,7 @@ namespace fleece { namespace impl { key(const key&) =delete; private: void setSharedKeys(SharedKeys*); - + slice const _rawString; SharedKeys* _sharedKeys {nullptr}; uint32_t _hint {0xFFFFFFFF}; diff --git a/Fleece/Core/Pointer.cc b/Fleece/Core/Pointer.cc index 835fee78..053e4fe4 100644 --- a/Fleece/Core/Pointer.cc +++ b/Fleece/Core/Pointer.cc @@ -101,10 +101,11 @@ namespace fleece { namespace impl { namespace internal { } - bool Pointer::validate(bool wide, const void *dataStart) const noexcept{ + bool Pointer::validate(bool wide, const void *dataStart, + bool checkSharedKeyExists) const noexcept{ const void *dataEnd = this; const Value *target = carefulDeref(wide, dataStart, dataEnd); - return target && target->validate(dataStart, dataEnd); + return target && target->validate(dataStart, dataEnd, checkSharedKeyExists); } diff --git a/Fleece/Core/Pointer.hh b/Fleece/Core/Pointer.hh index c2237773..2969bf7b 100644 --- a/Fleece/Core/Pointer.hh +++ b/Fleece/Core/Pointer.hh @@ -58,7 +58,8 @@ namespace fleece { namespace impl { namespace internal { const void* &dataEnd) const noexcept; - bool validate(bool wide, const void *dataStart) const noexcept FLPURE; + bool validate(bool wide, const void *dataStart, + bool checkSharedKeyExists) const noexcept FLPURE; private: // Byte offset as interpreted prior to the 'extern' flag diff --git a/Fleece/Core/SharedKeys.hh b/Fleece/Core/SharedKeys.hh index 057cb493..74be553f 100644 --- a/Fleece/Core/SharedKeys.hh +++ b/Fleece/Core/SharedKeys.hh @@ -75,7 +75,10 @@ namespace fleece { namespace impl { alloc_slice stateData() const; void writeState(Encoder &enc) const; - /** Sets the maximum length of string that can be mapped. (Defaults to 16 bytes.) */ + /** The maximum length of string that can be mapped. */ + size_t maxKeyLength() {return _maxKeyLength;} + + /** Sets maxKeyLength. (Defaults to 16 bytes.) */ void setMaxKeyLength(size_t m) {_maxKeyLength = m;} /** The number of stored keys. */ diff --git a/Fleece/Core/Value+Dump.cc b/Fleece/Core/Value+Dump.cc index 19493f01..c731f809 100644 --- a/Fleece/Core/Value+Dump.cc +++ b/Fleece/Core/Value+Dump.cc @@ -253,7 +253,7 @@ namespace fleece { namespace impl { bool Value::dump(slice data, std::ostream &out) { - auto root = fromData(data); + auto root = fromData(data, /*checkSharedKeyExists=*/ false); if (!root) return false; // Walk the tree and collect every value with its address: diff --git a/Fleece/Core/Value.cc b/Fleece/Core/Value.cc index 8955d780..acf1a6d8 100644 --- a/Fleece/Core/Value.cc +++ b/Fleece/Core/Value.cc @@ -16,6 +16,7 @@ #include "Dict.hh" #include "Internal.hh" #include "Doc.hh" +#include "SharedKeys.hh" #include "HeapValue.hh" #include "Endian.hh" #include "FleeceException.hh" @@ -316,9 +317,9 @@ namespace fleece { namespace impl { return findRoot(s); } - const Value* Value::fromData(slice s) noexcept { + const Value* Value::fromData(slice s, bool checkSharedKeyExists) noexcept { auto root = findRoot(s); - if (root && _usuallyFalse(!root->validate(s.buf, s.end()))) + if (root && _usuallyFalse(!root->validate(s.buf, s.end(), checkSharedKeyExists))) root = nullptr; return root; } @@ -344,38 +345,114 @@ namespace fleece { namespace impl { return root; } - bool Value::validate(const void *dataStart, const void *dataEnd) const noexcept { + bool Value::validate(const void *dataStart, const void *dataEnd, + bool checkSharedKeyExists) const noexcept { + // Check that size fits first so we don't read past the end of the buffer: + auto size = carefulDataSize(dataEnd); + if (_usuallyFalse(size == -1)) + // Size could not be read because the data is invalid. + return false; + if (_usuallyFalse(offsetby(this, size) > dataEnd)) + return false; + auto t = tag(); if (t == kArrayTag || t == kDictTag) { Array::impl array(this); if (_usuallyTrue(array._count > 0)) { + bool isDict = t == kDictTag; + bool isWide = array._width == kWide; + auto sharedKeys = t == kDictTag ? Scope::sharedKeys(this) : nullptr; + // For validation purposes a Dict is just an array with twice as many items: size_t itemCount = array._count; - if (_usuallyTrue(t == kDictTag)) + if (_usuallyTrue(isDict)) itemCount *= 2; - // Check that size fits: - auto itemsSize = itemCount * array._width; - if (_usuallyFalse(offsetby(array._first, itemsSize) > dataEnd)) - return false; // Check each Array/Dict element: auto item = array._first; - while (itemCount-- > 0) { + const Value* lastKey = nullptr; + bool isParentDict = false; + for (size_t i = 0; i < itemCount; ++i) { + bool isKey = isDict && i % 2 == 0; + bool isParentKey = false; + if (_usuallyFalse(isKey && i == 0 && Dict::isMagicParentKey(item))) { + isKey = false; + isParentKey = true; + isParentDict = true; + } + auto nextItem = offsetby(item, array._width); if (item->isPointer()) { - if (_usuallyFalse(!item->_asPointer()->validate(array._width == kWide, dataStart))) + if (_usuallyFalse(!item->_asPointer()->validate(isWide, dataStart, + checkSharedKeyExists))) return false; + item = item->deref(isWide); } else { - if (_usuallyFalse(!item->validate(dataStart, nextItem))) + if (_usuallyFalse(!item->validate(dataStart, nextItem, + checkSharedKeyExists))) + return false; + } + + if (_usuallyFalse(isKey && !item->isValidKey(sharedKeys, checkSharedKeyExists, + &lastKey, isWide))) + return false; + if (_usuallyFalse(!isParentKey && isParentDict)) { + if (_usuallyFalse(item->tag() != kDictTag)) return false; + isParentDict = false; } + item = nextItem; } return true; } } - // Default: just check that size fits: - return offsetby(this, dataSize()) <= dataEnd; + + return true; + } + + bool Value::isValidKey(SharedKeys* sharedKeys, bool checkSharedKeyExists, const Value** lastKey, + bool wide) const noexcept { + switch (tag()) { + case kStringTag: + if (sharedKeys) { + auto stringKey = asString(); + if (stringKey.size > sharedKeys->maxKeyLength()) + break; + int sharedKey; + if (sharedKeys->encode(stringKey, sharedKey)) + // Any key that has be encoded as a shared must always be encoded + // as a shared key. + return false; + } + break; + case kShortIntTag: { + auto key = asInt(); + // Shared keys must be non-negative. + if (key < 0) + return false; + + if (gDisableNecessarySharedKeysCheck) + checkSharedKeyExists = false; + + // SharedKeys must be available and key must exist in them. + if (checkSharedKeyExists && (!sharedKeys || !sharedKeys->decode(key))) + return false; + break; + } + default: + // Non-string keys are not allowed. + return false; + } + + if (*lastKey) { + // Keys must be sorted and unique. + if (compareKeys(this, *lastKey, wide) <= 0) + return false; + } + + *lastKey = this; + return true; } // This does not include the inline items in arrays/dicts @@ -394,6 +471,61 @@ namespace fleece { namespace impl { } } + // Returns the full size of the value, including items in arrays/dicts + // while taking care not to read past dataEnd. + // + // Returns -1 if the data is invalid and the size could not be read. + ptrdiff_t Value::carefulDataSize(const void* dataEnd) const noexcept { + auto t = tag(); + switch(t) { + case kStringTag: + case kBinaryTag: { + slice data(&_byte[1], tinyValue()); + if (_usuallyFalse(data.size == 0x0F)) { + // This means the actual length follows as a varint: + size_t maxDataLength = (uint8_t*)dataEnd - (uint8_t*)data.buf; + data = slice(data.buf, maxDataLength); + uint32_t length; + size_t lengthBytes = GetUVarInt32(data, &length); + if (_usuallyFalse(lengthBytes == 0)) + // Invalid varint so the data is invalid. + return -1; + return 1 + lengthBytes + length; + } + return 1 + data.size; + } + case kArrayTag: + case kDictTag: { + if (_usuallyFalse(isMutable())) + return -1; + + auto first = (const Value*)(&_byte[2]); + auto width = isWideArray() ? kWide : kNarrow; + auto count = countValue(); + size_t extraCountBytes = 0; + if (_usuallyFalse(count == kLongArrayCount)) { + // Long count is stored as a varint: + size_t maxDataLength = (uint8_t*)dataEnd - (uint8_t*)first; + uint32_t extraCount; + extraCountBytes = GetUVarInt32(slice(first, maxDataLength), &extraCount); + if (_usuallyFalse(extraCountBytes == 0)) + // Invalid varint so the data is invalid. + return -1; + count += extraCount; + first = offsetby(first, extraCountBytes + (extraCountBytes & 1)); + } + + size_t itemCount = count; + if (_usuallyTrue(t == kDictTag)) + itemCount *= 2; + size_t itemsSize = itemCount * width; + return 2 + extraCountBytes + itemsSize; + } + default: + return dataSize(); + } + } + #pragma mark - POINTERS: diff --git a/Fleece/Core/Value.hh b/Fleece/Core/Value.hh index 77b81d4b..c63756e3 100644 --- a/Fleece/Core/Value.hh +++ b/Fleece/Core/Value.hh @@ -65,7 +65,7 @@ namespace fleece { namespace impl { Validates the data first; if it's invalid, returns nullptr. Does NOT copy or take ownership of the data; the caller is responsible for keeping it intact. Any changes to the data will invalidate any FLValues obtained from it. */ - static const Value* fromData(slice) noexcept; + static const Value* fromData(slice, bool checkSharedKeyExists= true) noexcept; /** Returns a pointer to the root value in the encoded data, without validating. This is a lot faster, but "undefined behavior" occurs if the data is corrupt... */ @@ -206,7 +206,10 @@ namespace fleece { namespace impl { { } static const Value* findRoot(slice) noexcept FLPURE; - bool validate(const void* dataStart, const void *dataEnd) const noexcept FLPURE; + bool validate(const void* dataStart, const void *dataEnd, + bool checkSharedKeyExists) const noexcept FLPURE; + bool isValidKey(SharedKeys* sharedKeys, bool checkSharedKeyExists, const Value** lastKey, + bool wide) const noexcept FLPURE; internal::tags tag() const noexcept FLPURE {return (internal::tags)(_byte[0] >> 4);} unsigned tinyValue() const noexcept FLPURE {return _byte[0] & 0x0F;} @@ -239,6 +242,7 @@ namespace fleece { namespace impl { FLPURE const Value* next() const noexcept {return next(WIDE);} size_t dataSize() const noexcept FLPURE; + ptrdiff_t carefulDataSize(const void* dataEnd) const noexcept FLPURE; //////// Here's the data: diff --git a/Tests/SharedKeysTests.cc b/Tests/SharedKeysTests.cc index 7cdc27d4..bd6e61ee 100644 --- a/Tests/SharedKeysTests.cc +++ b/Tests/SharedKeysTests.cc @@ -19,6 +19,7 @@ using namespace std; using namespace fleece::impl; +using namespace fleece::impl::internal; TEST_CASE("basic", "[SharedKeys]") { @@ -472,9 +473,11 @@ TEST_CASE("big JSON encoding", "[SharedKeys]") { int nameKey; REQUIRE(sk->encode("name"_sl, nameKey)); + gDisableNecessarySharedKeysCheck = true; auto root = Value::fromTrustedData(encoded)->asArray(); auto person = root->get(33)->asDict(); const Value *name = person->get(nameKey); std::string nameStr = (std::string)name->asString(); REQUIRE(nameStr == std::string("Janet Ayala")); + gDisableNecessarySharedKeysCheck = false; } From 5729d30edfe88780c35fab509eec612cd1ae1ec9 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Wed, 28 Sep 2022 15:05:34 +0200 Subject: [PATCH 4/6] Handle binary and int values in `ValueSlot::copyValue` --- Fleece/Mutable/ValueSlot.cc | 7 +++++++ Tests/API_ValueTests.cc | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/Fleece/Mutable/ValueSlot.cc b/Fleece/Mutable/ValueSlot.cc index 591f5d22..804ddffc 100644 --- a/Fleece/Mutable/ValueSlot.cc +++ b/Fleece/Mutable/ValueSlot.cc @@ -257,6 +257,13 @@ namespace fleece { namespace impl { case kStringTag: set(value->asString()); break; + case kBinaryTag: + setData(value->asData()); + break; + case kIntTag: + if (value->isUnsigned()) set(value->asUnsigned()); + else set(value->asInt()); + break; case kFloatTag: set(value->asDouble()); break; diff --git a/Tests/API_ValueTests.cc b/Tests/API_ValueTests.cc index c6310f30..4ee2aa73 100644 --- a/Tests/API_ValueTests.cc +++ b/Tests/API_ValueTests.cc @@ -380,3 +380,31 @@ TEST_CASE("API Mutable copy of Dict with undefined value", "[API]") { CHECK(copy.count() == 1); CHECK(copy["a"].type() == kFLUndefined); } + +TEST_CASE("API Mutable copy of Dict with int", "[API]") { + auto enc = fleece::Encoder(); + enc.beginDict(); + enc.writeKey("a"); + enc.writeInt(0xFFFFFFFFFFFFFF); + enc.endDict(); + auto data = enc.finish(); + auto doc = Doc(data); + auto copy = doc.root().asDict().mutableCopy(kFLCopyImmutables); + CHECK(copy.count() == 1); + CHECK(copy["a"].type() == kFLNumber); + CHECK(copy["a"].asInt() == 0xFFFFFFFFFFFFFF); +} + +TEST_CASE("API Mutable copy of Dict with data", "[API]") { + auto enc = fleece::Encoder(); + enc.beginDict(); + enc.writeKey("a"); + enc.writeData("aaaaaaaaaaaaaaaa"_sl); + enc.endDict(); + auto data = enc.finish(); + auto doc = Doc(data); + auto copy = doc.root().asDict().mutableCopy(kFLCopyImmutables); + CHECK(copy.count() == 1); + CHECK(copy["a"].type() == kFLData); + CHECK(copy["a"].asData() == "aaaaaaaaaaaaaaaa"_sl); +} From a371d4bc86593b11765342f2ff98cd2e0498ed50 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Fri, 30 Sep 2022 23:10:49 +0200 Subject: [PATCH 5/6] Fix comparing `nullslice` and empty slice --- Fleece/API_Impl/FLSlice.cc | 19 ++++++++++++++++--- Tests/API_ValueTests.cc | 13 +++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Fleece/API_Impl/FLSlice.cc b/Fleece/API_Impl/FLSlice.cc index d6c1bc13..a4daed4a 100644 --- a/Fleece/API_Impl/FLSlice.cc +++ b/Fleece/API_Impl/FLSlice.cc @@ -41,15 +41,28 @@ FL_ASSUME_NONNULL_BEGIN __hot bool FLSlice_Equal(FLSlice a, FLSlice b) noexcept { - return a.size == b.size && FLMemCmp(a.buf, b.buf, a.size) == 0; + if (a.size == b.size) { + if (a.size == 0) + // Check wether both slices are the nullslice or empty slices. + return (a.buf == nullptr) == (b.buf == nullptr); + else + return FLMemCmp(a.buf, b.buf, a.size) == 0; + } + return false; } - __hot int FLSlice_Compare(FLSlice a, FLSlice b) noexcept { // Optimized for speed, not simplicity! if (a.size == b.size) - return FLMemCmp(a.buf, b.buf, a.size); + if (a.size == 0) { + // Sort the nullsice before an empty slice. + if ((a.buf == nullptr) == (b.buf == nullptr)) + return 0; + else + return a.buf == nullptr ? -1 : 1; + } else + return FLMemCmp(a.buf, b.buf, a.size); else if (a.size < b.size) { int result = FLMemCmp(a.buf, b.buf, a.size); return result ? result : -1; diff --git a/Tests/API_ValueTests.cc b/Tests/API_ValueTests.cc index 4ee2aa73..b571db18 100644 --- a/Tests/API_ValueTests.cc +++ b/Tests/API_ValueTests.cc @@ -408,3 +408,16 @@ TEST_CASE("API Mutable copy of Dict with data", "[API]") { CHECK(copy["a"].type() == kFLData); CHECK(copy["a"].asData() == "aaaaaaaaaaaaaaaa"_sl); } + +TEST_CASE("API Mutable copy of Dict with empty string shared key", "[API]") { + auto sk = fleece::SharedKeys::create(); + auto enc = fleece::Encoder(sk); + enc.beginDict(); + enc.writeKey(""); + enc.writeNull(); + enc.endDict(); + auto doc = enc.finishDoc(); + auto copy = doc.root().asDict().mutableCopy(kFLCopyImmutables); + CHECK(copy.count() == 1); + CHECK(copy[""].type() == kFLNull); +} From da31ee9a10151d0eacd44d1c02b61c65b457d4b6 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Sat, 1 Oct 2022 00:43:57 +0200 Subject: [PATCH 6/6] Fix count of dicts with parent --- Fleece/Core/Dict.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Fleece/Core/Dict.cc b/Fleece/Core/Dict.cc index 9a64586b..b568eade 100644 --- a/Fleece/Core/Dict.cc +++ b/Fleece/Core/Dict.cc @@ -309,7 +309,7 @@ namespace fleece { namespace impl { if (_usuallyFalse(isMutable())) return heapDict()->count(); Array::impl imp(this); - if (_usuallyFalse(imp._count > 1 && isMagicParentKey(imp._first))) { + if (_usuallyFalse(imp._count >= 1 && isMagicParentKey(imp._first))) { // Dict has a parent; this makes counting much more expensive! uint32_t c = 0; for (iterator i(this); i; ++i)