Future-proof data format used for LocalStorage.

content/renderer/dom_storage/local_storage_cached_area.cc

Index: content/renderer/dom_storage/local_storage_cached_area.cc
diff --git a/content/renderer/dom_storage/local_storage_cached_area.cc b/content/renderer/dom_storage/local_storage_cached_area.cc
index 77ab50b3df3bbe74cc0378a03532e5343b6054ce..93162ee2699bdb51b74b09f532786f1960551a69 100644
--- a/content/renderer/dom_storage/local_storage_cached_area.cc
+++ b/content/renderer/dom_storage/local_storage_cached_area.cc
@@ -24,14 +24,27 @@ namespace content {
 
 namespace {
 
+// Don't change or reorder any of the values in this enum, as these values
+// are serialized on disk.
+enum class StorageFormat : uint8_t { UTF16 = 0 };
+
 base::string16 Uint8VectorToString16(const std::vector<uint8_t>& input) {
-  return base::string16(reinterpret_cast<const base::char16*>(input.data()),
-                        input.size() / sizeof(base::char16));
+  CHECK(input.size() % sizeof(base::char16) == 1);
+  CHECK_EQ(input[0], static_cast<uint8_t>(StorageFormat::UTF16));

[michaeln, 2017/03/18 01:00:30]

Not sure we should crash the renderer here? Most users won't figure out how to
fix it.

For this to happen, either we downgrading to an older version of chrome or
there's corruption in database (assuming its not memory corruption of some
kind). Is that right? Is the version downgrade case detected farther upstream?

[Marijn Kruisselbrink, 2017/03/21 23:16:29]

Or compromised renderers, yes. So yeah, crashing the renderer is probably not
the right thing to do (even though that apparently is what indexeddb currently
does with data it can't deserialize). Changed it to for now just silently ignore
corrupt data, and added a TODO to do something better.

+  base::string16 result;
+  result.resize(input.size() / sizeof(base::char16));
+  std::memcpy(reinterpret_cast<void*>(&result[0]), input.data() + 1,

[michaeln, 2017/03/18 01:00:30]

is there a reason to use memcpy instead of a string16 ctor directly?

[Marijn Kruisselbrink, 2017/03/21 23:16:29]

I believe that using a string16 ctor directly would be potentially undefined
behavior, as it would involve reading a base::char16 from an address that is
almost certainly not correctly aligned for such a type. Having said that, I
believe that on all platforms and compilers we support it would actually work
correctly, but using memcpy seemed cleaner, since (I think) it's guaranteed to
work correctly.

+              input.size() - 1);
+  return result;
 }
 
 std::vector<uint8_t> String16ToUint8Vector(const base::string16& input) {
   const uint8_t* data = reinterpret_cast<const uint8_t*>(input.data());
-  return std::vector<uint8_t>(data, data + input.size() * sizeof(base::char16));
+  std::vector<uint8_t> result;
+  result.reserve(input.size() * sizeof(base::char16) + 1);
+  result.push_back(static_cast<uint8_t>(StorageFormat::UTF16));
+  result.insert(result.end(), data, data + input.size() * sizeof(base::char16));
+  return result;
 }
 
 class GetAllCallback : public mojom::LevelDBWrapperGetAllCallback {