From 8cdba4098758c6ca764559811760eebb2a3131f8 Mon Sep 17 00:00:00 2001
From: David Vacca <dvacca@fb.com>
Date: Wed, 21 Apr 2021 10:22:31 -0700
Subject: [PATCH] Fix MapBuffer.getString() method

Summary:
found a bug in MapBuffer.getString() method, this diff is fixing it

changelog: [internal] internal

Reviewed By: sammy-SC

Differential Revision: D27904644

fbshipit-source-id: 746812235539ff75c5ce53c6f872ede9779fa1fa
---
 .../react/renderer/mapbuffer/MapBuffer.cpp       | 12 +++++++-----
 .../renderer/mapbuffer/tests/MapBufferTest.cpp   | 16 ++++++++++++++--
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp b/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp
index 1821b14620..4829fcaaa9 100644
--- a/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp
+++ b/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp
@@ -12,6 +12,8 @@ using namespace facebook::react;
 namespace facebook {
 namespace react {
 
+// TODO T83483191: Extend MapBuffer C++ implementation to support basic random
+// access
 MapBuffer::MapBuffer(uint8_t *const data, uint16_t dataSize) {
   react_native_assert(
       (data != nullptr) && "Error trying to build an invalid MapBuffer");
@@ -75,21 +77,21 @@ std::string MapBuffer::getString(Key key) const {
   // of the map buffer
   int dynamicDataOffset = getDynamicDataOffset();
   int stringLength = 0;
+  int offset = getInt(key);
   memcpy(
       reinterpret_cast<uint8_t *>(&stringLength),
-      reinterpret_cast<const uint8_t *>(data_ + dynamicDataOffset),
+      reinterpret_cast<const uint8_t *>(data_ + dynamicDataOffset + offset),
       INT_SIZE);
 
-  int valueOffset = getInt(key) + sizeof(stringLength);
-
   char *value = new char[stringLength];
 
   memcpy(
       reinterpret_cast<char *>(value),
-      reinterpret_cast<const char *>(data_ + dynamicDataOffset + valueOffset),
+      reinterpret_cast<const char *>(
+          data_ + dynamicDataOffset + offset + INT_SIZE),
       stringLength);
 
-  return std::string(value);
+  return std::string(value, 0, stringLength);
 }
 
 MapBuffer MapBuffer::getMapBuffer(Key key) const {
diff --git a/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp b/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp
index 2578a73740..9dd36eeb6d 100644
--- a/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp
+++ b/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp
@@ -70,7 +70,8 @@ TEST(MapBufferTest, testNullEntries) {
   EXPECT_EQ(map.getCount(), 2);
   EXPECT_EQ(map.isNull(0), true);
   EXPECT_EQ(map.isNull(1), false);
-  // TODO T83483191: serialize null values to be distinguishable from '0' values
+  // TODO T83483191: serialize null values to be distinguishable from '0'
+  // values
   // EXPECT_EQ(map.isNull(1),  false);
   // EXPECT_EQ(map.getBool(1),  false);
 }
@@ -98,7 +99,7 @@ TEST(MapBufferTest, testStringEntries) {
   EXPECT_EQ(map.getString(0), "This is a test");
 }
 
-TEST(MapBufferTest, testUTFStringEntries) {
+TEST(MapBufferTest, testUTFStringEntry) {
   auto builder = MapBufferBuilder();
 
   builder.putString(0, "Let's count: 的, 一, 是");
@@ -107,6 +108,17 @@ TEST(MapBufferTest, testUTFStringEntries) {
   EXPECT_EQ(map.getString(0), "Let's count: 的, 一, 是");
 }
 
+TEST(MapBufferTest, testUTFStringEntries) {
+  auto builder = MapBufferBuilder();
+
+  builder.putString(0, "Let's count: 的, 一, 是");
+  builder.putString(1, "This is a test");
+  auto map = builder.build();
+
+  EXPECT_EQ(map.getString(0), "Let's count: 的, 一, 是");
+  EXPECT_EQ(map.getString(1), "This is a test");
+}
+
 TEST(MapBufferTest, testEmptyMap) {
   auto builder = MapBufferBuilder();
   auto map = builder.build();