Skip to content

Commit

Permalink
Safari's IndexedDB data may not be deserialized correctly after syste…
Browse files Browse the repository at this point in the history
…m upgrades

https://bugs.webkit.org/show_bug.cgi?id=266806
rdar://120031024

Reviewed by Mark Lam.

To fix rdar://119834827, we introduce version 12.1 to SerializeScriptValue, which changed the terminator of the indexed
property section in array compared to version 12. To make sure deserializer knows to deserialize version 12.1, we encode
the minor version in the highest 8 bits of version number. We keep the lowest 24 bit as major version number for
backward compatibility (the previously stored 32-bit major version number can be intepreted as major version with minor
version 0).

* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::majorVersionFor):
(WebCore::minorVersionFor):
(WebCore::makeVersion):
(WebCore::currentVersion):
(WebCore::CloneSerializer::serialize):
(WebCore::CloneSerializer::CloneSerializer):
(WebCore::CloneDeserializer::deserializeString):
(WebCore::CloneDeserializer::deserialize):
(WebCore::CloneDeserializer::isValid const):
(WebCore::CloneDeserializer::shouldRetryWithVersionUpgrade):
(WebCore::CloneDeserializer::upgradeVersion):
(WebCore::CloneDeserializer::read):
(WebCore::CloneDeserializer::readFile):
(WebCore::CloneDeserializer::readArrayBuffer):
(WebCore::CloneDeserializer::readArrayBufferView):
(WebCore::CloneDeserializer::readImageBitmap):
(WebCore::CloneDeserializer::readTerminal):
(WebCore::CloneDeserializer::version const): Deleted.
(WebCore::SerializedScriptValue::wireFormatVersion): Deleted.
* Source/WebCore/bindings/js/SerializedScriptValue.h:

Originally-landed-as: 272448.79@safari-7618-branch (7a1bb1a). rdar://124556752
Canonical link: https://commits.webkit.org/276710@main
  • Loading branch information
szewai authored and JonWBedard committed Mar 26, 2024
1 parent 6cf8742 commit 3e904eb
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 37 deletions.
102 changes: 67 additions & 35 deletions Source/WebCore/bindings/js/SerializedScriptValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -690,9 +690,17 @@ enum class CryptoKeyOKPOpNameTag {
ED25519 = 1,
};
const uint8_t cryptoKeyOKPOpNameTagMaximumValue = 1;


/* CurrentVersion tracks the serialization version so that persistent stores
static constexpr unsigned CurrentMajorVersion = 15;
static constexpr unsigned CurrentMinorVersion = 0;
static constexpr unsigned majorVersionFor(unsigned version) { return version & 0x00FFFFFF; }
static constexpr unsigned minorVersionFor(unsigned version) { return version >> 24; }
static constexpr unsigned makeVersion(unsigned major, unsigned minor)
{
ASSERT_UNDER_CONSTEXPR_CONTEXT(major < (1u << 24));
ASSERT_UNDER_CONSTEXPR_CONTEXT(minor < (1u << 8));
return (minor << 24) | major;
}
/* currentVersion tracks the serialization version so that persistent stores
* are able to correctly bail out in the case of encountering newer formats.
*
* Initial version was 1.
Expand All @@ -708,11 +716,12 @@ const uint8_t cryptoKeyOKPOpNameTagMaximumValue = 1;
* Version 10. changed the length (and offsets) of ArrayBuffers (and ArrayBufferViews) from 32 to 64 bits.
* Version 11. added support for Blob's memory cost.
* Version 12. added support for agent cluster ID.
* Version 12.1. changed the terminator of the indexed property section in array.
* Version 13. added support for ErrorInstance objects.
* Version 14. encode booleans as uint8_t instead of int32_t.
* Version 15. changed the terminator of the indexed property section in array.
*/
static constexpr unsigned CurrentVersion = 15;
static constexpr unsigned currentVersion() { return makeVersion(CurrentMajorVersion, CurrentMinorVersion); }
static constexpr unsigned TerminatorTag = 0xFFFFFFFF;
static constexpr unsigned StringPoolTag = 0xFFFFFFFE;
static constexpr unsigned NonIndexPropertiesTag = 0xFFFFFFFD;
Expand All @@ -731,7 +740,7 @@ static_assert(TerminatorTag > MAX_ARRAY_INDEX);
* minimum sized unsigned integer type required to represent the maximum index
* in the constant pool.
*
* SerializedValue :- <CurrentVersion:uint32_t> Value
* SerializedValue :- <version:uint32_t> Value
* Value :- Array | Object | Map | Set | Terminal
*
* Array :-
Expand Down Expand Up @@ -1085,7 +1094,7 @@ class CloneSerializer : public CloneBase {

static bool serialize(StringView string, Vector<uint8_t>& out)
{
writeLittleEndian(out, CurrentVersion);
writeLittleEndian(out, currentVersion());
if (string.isEmpty()) {
writeLittleEndian<uint8_t>(out, EmptyStringTag);
return true;
Expand Down Expand Up @@ -1160,7 +1169,7 @@ class CloneSerializer : public CloneBase {
#endif
, m_forStorage(forStorage)
{
write(CurrentVersion);
write(currentVersion());
fillTransferMap(messagePorts, m_transferredMessagePorts);
fillTransferMap(arrayBuffers, m_transferredArrayBuffers);
fillTransferMap(imageBitmaps, m_transferredImageBitmaps);
Expand Down Expand Up @@ -2964,7 +2973,7 @@ class CloneDeserializer : public CloneBase {
const uint8_t* ptr = buffer.begin();
const uint8_t* end = buffer.end();
uint32_t version;
if (!readLittleEndian(ptr, end, version) || version > CurrentVersion)
if (!readLittleEndian(ptr, end, version) || majorVersionFor(version) > CurrentMajorVersion)
return String();
uint8_t tag;
if (!readLittleEndian(ptr, end, tag) || tag != StringTag)
Expand Down Expand Up @@ -3041,7 +3050,7 @@ class CloneDeserializer : public CloneBase {

auto result = deserializer.deserialize();
// Deserialize again if data may have wrong version number, see rdar://118775332.
if (UNLIKELY(result.second != SerializationReturnCode::SuccessfullyCompleted && deserializer.version() == 14)) {
if (UNLIKELY(result.second != SerializationReturnCode::SuccessfullyCompleted && deserializer.shouldRetryWithVersionUpgrade())) {
CloneDeserializer newDeserializer(lexicalGlobalObject, globalObject, messagePorts, arrayBufferContentsArray, buffer, blobURLs, blobFilePaths, sharedBuffers, deserializer.takeDetachedImageBitmaps()
#if ENABLE(OFFSCREEN_CANVAS_IN_WORKERS)
, deserializer.takeDetachedOffscreenCanvases()
Expand Down Expand Up @@ -3146,7 +3155,8 @@ class CloneDeserializer : public CloneBase {
, m_canCreateDOMObject(m_isDOMGlobalObject && !globalObject->inherits<JSIDBSerializationGlobalObject>())
, m_ptr(buffer.data())
, m_end(buffer.data() + buffer.size())
, m_version(0xFFFFFFFF)
, m_majorVersion(0xFFFFFFFF)
, m_minorVersion(0xFFFFFFFF)
, m_messagePorts(messagePorts)
, m_arrayBufferContents(arrayBufferContents)
, m_arrayBuffers(arrayBufferContents ? arrayBufferContents->size() : 0)
Expand Down Expand Up @@ -3185,8 +3195,11 @@ class CloneDeserializer : public CloneBase {
, m_mediaStreamTracks(m_serializedMediaStreamTracks.size())
#endif
{
if (!read(m_version))
m_version = 0xFFFFFFFF;
unsigned version;
if (read(version)) {
m_majorVersion = majorVersionFor(version);
m_minorVersion = minorVersionFor(version);
}
}

CloneDeserializer(JSGlobalObject* lexicalGlobalObject, JSGlobalObject* globalObject, const Vector<RefPtr<MessagePort>>& messagePorts, ArrayBufferContentsArray* arrayBufferContents, const Vector<uint8_t>& buffer, const Vector<String>& blobURLs, const Vector<String> blobFilePaths, ArrayBufferContentsArray* sharedBuffers, Vector<std::optional<DetachedImageBitmap>>&& detachedImageBitmaps
Expand Down Expand Up @@ -3221,7 +3234,8 @@ class CloneDeserializer : public CloneBase {
, m_canCreateDOMObject(m_isDOMGlobalObject && !globalObject->inherits<JSIDBSerializationGlobalObject>())
, m_ptr(buffer.data())
, m_end(buffer.data() + buffer.size())
, m_version(0xFFFFFFFF)
, m_majorVersion(0xFFFFFFFF)
, m_minorVersion(0xFFFFFFFF)
, m_messagePorts(messagePorts)
, m_arrayBufferContents(arrayBufferContents)
, m_arrayBuffers(arrayBufferContents ? arrayBufferContents->size() : 0)
Expand Down Expand Up @@ -3263,8 +3277,11 @@ class CloneDeserializer : public CloneBase {
, m_mediaStreamTracks(m_serializedMediaStreamTracks.size())
#endif
{
if (!read(m_version))
m_version = 0xFFFFFFFF;
unsigned version;
if (read(version)) {
m_majorVersion = majorVersionFor(version);
m_minorVersion = minorVersionFor(version);
}
}

enum class VisitNamedMemberResult : uint8_t { Error, Break, Start, Unknown };
Expand Down Expand Up @@ -3330,12 +3347,31 @@ class CloneDeserializer : public CloneBase {
Vector<RefPtr<DetachedMediaSourceHandle>> takeDetachedMediaSourceHandles() { return std::exchange(m_detachedMediaSourceHandles, { }); }
#endif

bool isValid() const { return m_version <= CurrentVersion; }
unsigned version() const { return m_version; }
bool isValid() const
{
if (m_majorVersion > CurrentMajorVersion)
return false;
if (m_majorVersion == 12)
return m_minorVersion <= 1;
return !m_minorVersion;
}
bool shouldRetryWithVersionUpgrade()
{
if (m_majorVersion == 14 && !m_minorVersion)
return true;
if (m_majorVersion == 12 && !m_minorVersion)
return true;
return false;
}
void upgradeVersion()
{
RELEASE_ASSERT(m_version == 14);
++m_version;
ASSERT(shouldRetryWithVersionUpgrade());
if (m_majorVersion == 14 && !m_minorVersion) {
m_majorVersion = 15;
return;
}
if (m_majorVersion == 12 && !m_minorVersion)
m_minorVersion = 1;
}

template<SerializationTag tag>
Expand Down Expand Up @@ -3389,7 +3425,7 @@ class CloneDeserializer : public CloneBase {
enum class ForceReadingAs8Bit : bool { No, Yes };
bool read(bool& b, ForceReadingAs8Bit forceReadingAs8Bit = ForceReadingAs8Bit::No)
{
if (m_version >= 14 || forceReadingAs8Bit == ForceReadingAs8Bit::Yes) {
if (m_majorVersion >= 14 || forceReadingAs8Bit == ForceReadingAs8Bit::Yes) {
uint8_t integer;
if (!read(integer) || integer > 1)
return false;
Expand Down Expand Up @@ -3608,7 +3644,7 @@ class CloneDeserializer : public CloneBase {
if (!readStringData(name))
return false;
std::optional<int64_t> optionalLastModified;
if (m_version > 6) {
if (m_majorVersion > 6) {
double lastModified;
if (!read(lastModified))
return false;
Expand Down Expand Up @@ -3645,7 +3681,7 @@ class CloneDeserializer : public CloneBase {

bool readArrayBuffer(RefPtr<ArrayBuffer>& arrayBuffer)
{
if (m_version < 10)
if (m_majorVersion < 10)
return readArrayBufferImpl<uint32_t>(arrayBuffer);
return readArrayBufferImpl<uint64_t>(arrayBuffer);
}
Expand Down Expand Up @@ -3748,7 +3784,7 @@ class CloneDeserializer : public CloneBase {

bool readArrayBufferView(VM& vm, JSValue& arrayBufferView)
{
if (m_version < 10)
if (m_majorVersion < 10)
return readArrayBufferViewImpl<uint32_t>(vm, arrayBufferView);
return readArrayBufferViewImpl<uint64_t>(vm, arrayBufferView);
}
Expand Down Expand Up @@ -4634,7 +4670,7 @@ class CloneDeserializer : public CloneBase {
auto colorSpace = DestinationColorSpace::SRGB();
RefPtr<ArrayBuffer> arrayBuffer;

if (!read(rawFlags) || !read(logicalWidth) || !read(logicalHeight) || !read(resolutionScale) || (m_version > 8 && !read(colorSpace)) || !readArrayBufferImpl<uint32_t>(arrayBuffer)) {
if (!read(rawFlags) || !read(logicalWidth) || !read(logicalHeight) || !read(resolutionScale) || (m_majorVersion > 8 && !read(colorSpace)) || !readArrayBufferImpl<uint32_t>(arrayBuffer)) {
SERIALIZE_TRACE("FAIL deserialize");
fail();
return JSValue();
Expand Down Expand Up @@ -4905,7 +4941,7 @@ class CloneDeserializer : public CloneBase {
m_ptr += length;

auto resultColorSpace = PredefinedColorSpace::SRGB;
if (m_version > 7) {
if (m_majorVersion > 7) {
if (!read(resultColorSpace))
return JSValue();
}
Expand Down Expand Up @@ -4943,7 +4979,7 @@ class CloneDeserializer : public CloneBase {
if (!read(size))
return JSValue();
uint64_t memoryCost = 0;
if (m_version >= 11 && !read(memoryCost))
if (m_majorVersion >= 11 && !read(memoryCost))
return JSValue();
if (!m_canCreateDOMObject)
return jsNull();
Expand Down Expand Up @@ -5054,7 +5090,7 @@ class CloneDeserializer : public CloneBase {
}
#if ENABLE(WEBASSEMBLY)
case WasmModuleTag: {
if (m_version >= 12) {
if (m_majorVersion >= 12) {
// https://webassembly.github.io/spec/web-api/index.html#serialization
CachedStringRef agentClusterID;
bool agentClusterIDSuccessfullyRead = readStringData(agentClusterID);
Expand All @@ -5080,7 +5116,7 @@ class CloneDeserializer : public CloneBase {
return result;
}
case WasmMemoryTag: {
if (m_version >= 12) {
if (m_majorVersion >= 12) {
CachedStringRef agentClusterID;
bool agentClusterIDSuccessfullyRead = readStringData(agentClusterID);
if (!agentClusterIDSuccessfullyRead || agentClusterID->string() != agentClusterIDFromGlobalObject(*m_globalObject)) {
Expand Down Expand Up @@ -5302,7 +5338,8 @@ class CloneDeserializer : public CloneBase {
const bool m_canCreateDOMObject;
const uint8_t* m_ptr;
const uint8_t* const m_end;
unsigned m_version;
unsigned m_majorVersion;
unsigned m_minorVersion;
Vector<CachedString> m_constantPool;
Vector<Ref<ImageData>> m_imageDataPool;
const Vector<RefPtr<MessagePort>>& m_messagePorts;
Expand Down Expand Up @@ -5405,7 +5442,7 @@ DeserializationResult CloneDeserializer::deserialize()
goto error;
}

if (m_version >= 15) {
if (m_majorVersion >= 15 || (m_majorVersion == 12 && m_minorVersion == 1)) {
if (index == TerminatorTag) {
// We reached the end of the indexed properties section.
if (!read(index)) {
Expand Down Expand Up @@ -6367,11 +6404,6 @@ Ref<SerializedScriptValue> SerializedScriptValue::nullValue()
return adoptRef(*new SerializedScriptValue(Vector<uint8_t>()));
}

uint32_t SerializedScriptValue::wireFormatVersion()
{
return CurrentVersion;
}

Vector<String> SerializedScriptValue::blobURLs() const
{
return m_internals.blobHandles.map([](auto& handle) {
Expand Down
2 changes: 0 additions & 2 deletions Source/WebCore/bindings/js/SerializedScriptValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ class SerializedScriptValue : public ThreadSafeRefCounted<SerializedScriptValue>
WEBCORE_EXPORT JSC::JSValue deserialize(JSC::JSGlobalObject&, JSC::JSGlobalObject*, const Vector<RefPtr<MessagePort>>&, SerializationErrorMode = SerializationErrorMode::Throwing, bool* didFail = nullptr);
JSC::JSValue deserialize(JSC::JSGlobalObject&, JSC::JSGlobalObject*, const Vector<RefPtr<MessagePort>>&, const Vector<String>& blobURLs, const Vector<String>& blobFilePaths, SerializationErrorMode = SerializationErrorMode::Throwing, bool* didFail = nullptr);

static uint32_t wireFormatVersion();

WEBCORE_EXPORT String toString() const;

// API implementation helpers. These don't expose special behavior for ArrayBuffers or MessagePorts.
Expand Down

0 comments on commit 3e904eb

Please sign in to comment.