Avoid unnecessary byte-swapping in blink's SerializedScriptValue.

third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 17 matching lines @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "bindings/core/v8/SerializedScriptValue.h"
 
 #include <memory>
 #include "bindings/core/v8/DOMDataStore.h"
 #include "bindings/core/v8/DOMWrapperWorld.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/SerializationTag.h"
 #include "bindings/core/v8/SerializedScriptValueFactory.h"
 #include "bindings/core/v8/Transferables.h"
 #include "bindings/core/v8/V8ArrayBuffer.h"
 #include "bindings/core/v8/V8ImageBitmap.h"
 #include "bindings/core/v8/V8MessagePort.h"
 #include "bindings/core/v8/V8OffscreenCanvas.h"
 #include "bindings/core/v8/V8SharedArrayBuffer.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMSharedArrayBuffer.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/MessagePort.h"
 #include "core/frame/ImageBitmap.h"
 #include "platform/SharedBuffer.h"
 #include "platform/blob/BlobData.h"
 #include "platform/heap/Handle.h"
 #include "platform/wtf/Assertions.h"
 #include "platform/wtf/ByteOrder.h"
 #include "platform/wtf/PtrUtil.h"
 #include "platform/wtf/Vector.h"
+#include "platform/wtf/dtoa/utils.h"
 #include "platform/wtf/text/StringBuffer.h"
 #include "platform/wtf/text/StringHash.h"
 
 namespace blink {
 
 PassRefPtr<SerializedScriptValue> SerializedScriptValue::Serialize(
     v8::Isolate* isolate,
     v8::Local<v8::Value> value,
     const SerializeOptions& options,
     ExceptionState& exception) {
@@ skipping 15 matching lines @@
 
 PassRefPtr<SerializedScriptValue> SerializedScriptValue::Create() {
   return AdoptRef(new SerializedScriptValue);
 }
 
 PassRefPtr<SerializedScriptValue> SerializedScriptValue::Create(
     const String& data) {
   return AdoptRef(new SerializedScriptValue(data));
 }
 
+// Versions 16 and below (prior to April 2017) used ntohs() to byte-swap SSV
+// data when converting it to the wire format. This was a historical accient.
+//
+// As IndexedDB stores SSVs to disk indefinitely, we still need to keep around
+// the code needed to deserialize the old format.
+inline static bool IsByteSwappedWiredData(const char* data, size_t length) {
+  // TODO(pwnall): Return false early if we're on big-endian hardware. Chromium
+  // doesn't currently support big-endian hardware, and there's no header
+  // exposing endianness to Blink yet. ARCH_CPU_LITTLE_ENDIAN seems promising,
+  // but Blink is not currently allowed to include files from build/.
+
+  // The first SSV version without byte-swapping has two envelopes (Blink, V8),
+  // each of which is at least 2 bytes long.
+  if (length < 4)
+    return true;
+
+  // This code handles the following cases:
+  //
+  // v0 (byte-swapped)    - [d,    t,    ...], t = tag byte, d = first data byte
+  // v1-16 (byte-swapped) - [v,    0xFF, ...], v = version (1 <= v <= 16)
+  // v17+                 - [0xFF, v,    ...], v = first byte of version varint
+
+  if (static_cast<uint8_t>(data[0]) == kVersionTag) {
+    // The only case where byte-swapped data can have 0xFF in byte zero is
+    // version 0. This can only happen if byte one is a tag (supported in
+    // version 0) that takes in extra data, and the first byte of extra data is
+    // 0xFF. There are 13 such tags, listed below. These tags cannot be used as
+    // version numbers in the Blink-side SSV envelope.
+    //
+    //  35 - 0x23 - # - ImageDataTag
+    //  64 - 0x40 - @ - SparseArrayTag
+    //  68 - 0x44 - D - DateTag
+    //  73 - 0x49 - I - Int32Tag
+    //  78 - 0x4E - N - NumberTag
+    //  82 - 0x52 - R - RegExpTag
+    //  83 - 0x53 - S - StringTag
+    //  85 - 0x55 - U - Uint32Tag
+    //  91 - 0x5B - [ - ArrayTag
+    //  98 - 0x62 - b - BlobTag
+    // 102 - 0x66 - f - FileTag
+    // 108 - 0x6C - l - FileListTag
+    // 123 - 0x7B - { - ObjectTag
+    //
+    // Why we care about version 0:
+    //
+    // IndexedDB stores values using the SSV format. Currently, IndexedDB does
+    // not do any sort of migration, so a value written with a SSV version will
+    // be stored with that version until it is removed via an update or delete.
+    //
+    // IndexedDB was shipped in Chrome 11, which was released on April 27, 2011.
+    // SSV version 1 was added in WebKit r91698, which was shipped in Chrome 14,
+    // which was released on September 16, 2011.
+    static_assert(
+        SerializedScriptValue::kWireFormatVersion != 35 &&
+            SerializedScriptValue::kWireFormatVersion != 64 &&
+            SerializedScriptValue::kWireFormatVersion != 68 &&
+            SerializedScriptValue::kWireFormatVersion != 73 &&
+            SerializedScriptValue::kWireFormatVersion != 78 &&
+            SerializedScriptValue::kWireFormatVersion != 82 &&
+            SerializedScriptValue::kWireFormatVersion != 83 &&
+            SerializedScriptValue::kWireFormatVersion != 85 &&
+            SerializedScriptValue::kWireFormatVersion != 91 &&
+            SerializedScriptValue::kWireFormatVersion != 98 &&
+            SerializedScriptValue::kWireFormatVersion != 102 &&
+            SerializedScriptValue::kWireFormatVersion != 108 &&
+            SerializedScriptValue::kWireFormatVersion != 123,
+        "Using a burned version will prevent us from reading SSV version 0");
+
+    // Fast path until the Blink-side SSV envelope reaches version 35.
+    if (SerializedScriptValue::kWireFormatVersion < 35) {
+      if (static_cast<uint8_t>(data[1]) < 35)
+        return false;
+
+      // TODO(pwnall): Add UMA metric here.
+      return true;
+    }
+
+    // Slower path that would kick in after version 35, assuming we don't remove
+    // support for SSV version 0 by then.
+    static constexpr uint8_t version0Tags[] = {35, 64, 68, 73,  78,  82, 83,
+                                               85, 91, 98, 102, 108, 123};
+    return std::find(std::begin(version0Tags), std::end(version0Tags),
+                     data[1]) != std::end(version0Tags);
+  }
+
+  if (static_cast<uint8_t>(data[1]) == kVersionTag) {
+    // The last SSV format that used byte-flipping was version 16. The version
+    // number is stored (before byte-flipping) after a serialization tag, which
+    // is 0xFF.
+    return static_cast<uint8_t>(data[0]) != kVersionTag;
+  }
+
+  // If kVersionTag isn't in any of the first two bytes, this is SSV version 0,
+  // which was byte-flipped.

[jbroman, 2017/04/12 18:34:04]

change this comment and others to say "swapped" rather than "flipped", for
consistency?

[pwnall, 2017/04/12 21:37:30]

Done.
Thanks for catching this! I did a find-replace and got rid of a few instances
that managed to hide from me.

+  return true;
+}
+
 PassRefPtr<SerializedScriptValue> SerializedScriptValue::Create(
     const char* data,
     size_t length) {
   if (!data)
     return Create();
 
-  // Decode wire data from big endian to host byte order.
   DCHECK(!(length % sizeof(UChar)));
+  const UChar* src = reinterpret_cast<const UChar*>(data);
   size_t string_length = length / sizeof(UChar);
-  StringBuffer<UChar> buffer(string_length);
-  const UChar* src = reinterpret_cast<const UChar*>(data);
-  UChar* dst = buffer.Characters();
-  for (size_t i = 0; i < string_length; i++)
-    dst[i] = ntohs(src[i]);
 
-  return AdoptRef(new SerializedScriptValue(String::Adopt(buffer)));
+  if (IsByteSwappedWiredData(data, length)) {
+    // Decode wire data from big endian to host byte order.
+    StringBuffer<UChar> buffer(string_length);
+    UChar* dst = buffer.Characters();
+    for (size_t i = 0; i < string_length; ++i)
+      dst[i] = ntohs(src[i]);
+
+    return AdoptRef(new SerializedScriptValue(String::Adopt(buffer)));
+  }
+
+  return AdoptRef(new SerializedScriptValue(String(src, string_length)));
 }
 
 SerializedScriptValue::SerializedScriptValue()
     : has_registered_external_allocation_(false),
       transferables_need_external_allocation_registration_(false) {}
 
 SerializedScriptValue::SerializedScriptValue(const String& wire_data)
     : has_registered_external_allocation_(false),
       transferables_need_external_allocation_registration_(false) {
   size_t byte_length = wire_data.length() * 2;
   data_buffer_.reset(static_cast<uint8_t*>(WTF::Partitions::BufferMalloc(
       byte_length, "SerializedScriptValue buffer")));
   data_buffer_size_ = byte_length;
   wire_data.CopyTo(reinterpret_cast<UChar*>(data_buffer_.get()), 0,
                    wire_data.length());
 }
 
 SerializedScriptValue::~SerializedScriptValue() {
   // If the allocated memory was not registered before, then this class is
   // likely used in a context other than Worker's onmessage environment and the
   // presence of current v8 context is not guaranteed. Avoid calling v8 then.
   if (has_registered_external_allocation_) {
     ASSERT(v8::Isolate::GetCurrent());
     v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
         -static_cast<int64_t>(DataLengthInBytes()));
   }
 }
 
 PassRefPtr<SerializedScriptValue> SerializedScriptValue::NullValue() {
-  // UChar rather than uint8_t here to get host endian behavior.
-  static const UChar kNullData[] = {0xff09, 0x3000};
+  static const uint8_t kNullData[] = {
+      kVersionTag, kWireFormatVersion,  // Blink envelope

[jbroman, 2017/04/12 18:34:04]

I don't strenuously object, but this level of detail isn't really necessary
because we expect to be able to decode old versions into the distant future (and
there's a test that this value deserializes properly, so if that changes we'll
know).

Using the current wire format version might actually make it more brittle,
because it means this code will need to change if we change the format in a way
that makes this not the way null is written. So I think having a fixed 4-6 byte
thing with the comment "this is how we wrote null at some point in the past" is
probably enough.

[pwnall, 2017/04/12 21:37:30]

Done.

I'll explain my reasoning for the previous approach, and I can bring it back in
a follow-up patch if the explanation changes your mind. FWIW, there is a minor
performance advantage to using the current wire version -- it can make the
branch predictor happier.

While I agree that hard-wiring the Blink-side version can make the code a bit
more brittle, I think the opportunity is quite low, because the V8 serialization
version is fixed. There's still the possibility that the whole wrapper changes
with a Blink version bump, but we don't seem to do that very often.

+      // 13 is v8::ValueSerializer::GetCurrentDataFormatVersion(). Having a
+      // hard-coded version here is better than getting V8's version directly.
+      // This approach lets v8 make drastic changes to its serialization format
+      // without a 3-sided patch, as long as it retains the ability to
+      // deserialize the version number listed here.
+      kVersionTag, 13,  // V8 envelope
+      '0',              // kNull in v8::internal::SerializationTag
+      0x00};            // Padding to an integral number of UChars
   return Create(reinterpret_cast<const char*>(kNullData), sizeof(kNullData));
 }
 
 String SerializedScriptValue::ToWireString() const {
   // Add the padding '\0', but don't put it in |m_dataBuffer|.
   // This requires direct use of uninitialized strings, though.
   UChar* destination;
   size_t string_size_bytes = (data_buffer_size_ + 1) & ~1;
   String wire_string =
       String::CreateUninitialized(string_size_bytes / 2, destination);
   memcpy(destination, data_buffer_.get(), data_buffer_size_);
   if (string_size_bytes > data_buffer_size_)
     reinterpret_cast<char*>(destination)[string_size_bytes - 1] = '\0';
   return wire_string;
 }
 
-// Convert serialized string to big endian wire data.
 void SerializedScriptValue::ToWireBytes(Vector<char>& result) const {
   DCHECK(result.IsEmpty());
 
-  size_t wire_size_bytes = (data_buffer_size_ + 1) & ~1;
-  result.Resize(wire_size_bytes);
+  size_t result_size = (data_buffer_size_ + 1) & ~1;
+  result.Resize(result_size);
+  memcpy(result.Data(), data_buffer_.get(), data_buffer_size_);
 
-  const UChar* src = reinterpret_cast<UChar*>(data_buffer_.get());
-  UChar* dst = reinterpret_cast<UChar*>(result.Data());
-  for (size_t i = 0; i < data_buffer_size_ / 2; i++)
-    dst[i] = htons(src[i]);
-
-  // This is equivalent to swapping the byte order of the two bytes (x, 0),
-  // depending on endianness.
-  if (data_buffer_size_ % 2)
-    dst[wire_size_bytes / 2 - 1] = data_buffer_[data_buffer_size_ - 1] << 8;
+  if (result_size > data_buffer_size_) {
+    DCHECK_EQ(result_size, data_buffer_size_ + 1);
+    result[data_buffer_size_] = 0;
+  }
 }
 
 static void AccumulateArrayBuffersForAllWorlds(
     v8::Isolate* isolate,
     DOMArrayBuffer* object,
     Vector<v8::Local<v8::ArrayBuffer>, 4>& buffers) {
   Vector<RefPtr<DOMWrapperWorld>> worlds;
   DOMWrapperWorld::AllWorldsInCurrentThread(worlds);
   for (const auto& world : worlds) {
     v8::Local<v8::Object> wrapper = world->DomDataStore().Get(object, isolate);
@@ skipping 281 matching lines @@
   // Only (re)register allocation cost for transferables if this
   // SerializedScriptValue has explicitly unregistered them before.
   if (array_buffer_contents_array_ &&
       transferables_need_external_allocation_registration_) {
     for (auto& buffer : *array_buffer_contents_array_)
       buffer.RegisterExternalAllocationWithCurrentContext();
   }
 }
 
 }  // namespace blink