Look up props read out of order by name in RawPropsParser

packages/react-native/ReactCommon/react/renderer/core/RawPropsParser.cpp

@@ -56,43 +56,32 @@ const RawValue* RawPropsParser::at(
     return nullptr;
   }
 
-// Normally, keys are looked up in-order. For performance we can simply
-// increment this key counter, and if the key is equal to the key at the next
-// index, there's no need to do any lookups. However, it's possible for keys
-// to be accessed out-of-order or multiple times, in which case we start
-// searching again from index 0.
-// To prevent infinite loops (which can occur if you look up a key that
-// doesn't exist) we keep track of whether or not we've already looped around,
-// and log and return nullptr if so. However, we ONLY do this in debug mode,
-// where you're more likely to look up a nonexistent key as part of debugging.
-// You can (and must) ensure infinite loops are not possible in production by:
-// (1) constructing all props objects without conditionals, or (2) if there
-// are conditionals, ensure that in the parsing setup case, the Props
-// constructor will access _all_ possible props. To ensure this performance
-// optimization is utilized, always access props in the same order every time.
-// This is trivial if you have a simple Props constructor, but difficult or
-// impossible if you have a shared sub-prop Struct that is used by multiple
-// parent Props.
-#ifdef REACT_NATIVE_DEBUG
-  bool resetLoop = false;
-#endif
-  do {
-    rawProps.keyIndexCursor_++;
+  // Normally, keys are looked up in-order. For performance we can simply
+  // increment this key counter, and if the key is equal to the key at the next
+  // index, there's no need to do any lookups. However, it's possible for keys
+  // to be accessed out-of-order or multiple times, in which case we find the
+  // key's index in `nameToIndex_` and continue from there.
+  rawProps.keyIndexCursor_++;
+
+  if (static_cast<size_t>(rawProps.keyIndexCursor_) >= keys_.size())
+      [[unlikely]] {
+    rawProps.keyIndexCursor_ = 0;
+  }
+
+  if (!keysEqual(key, keys_[rawProps.keyIndexCursor_])) {
+    auto keyIndex = nameToIndex_.at(key);
 
-    if (static_cast<size_t>(rawProps.keyIndexCursor_) >= keys_.size())
-        [[unlikely]] {
+    if (keyIndex == kRawPropsValueIndexEmpty) [[unlikely]] {
 #ifdef REACT_NATIVE_DEBUG
-      if (resetLoop) {
-        LOG(ERROR)
-            << "Looked up property name which was not seen when preparing: "
-            << key;
-        return nullptr;
-      }
-      resetLoop = true;
+      LOG(ERROR)
+          << "Looked up property name which was not seen when preparing: "
+          << key;
 #endif
-      rawProps.keyIndexCursor_ = 0;
+      return nullptr;
     }
-  } while (!keysEqual(key, keys_[rawProps.keyIndexCursor_]));
+
+    rawProps.keyIndexCursor_ = keyIndex;
+  }
 
   auto valueIndex = rawProps.keyIndexToValueIndex_[rawProps.keyIndexCursor_];
   return valueIndex == kRawPropsValueIndexEmpty ? nullptr

packages/react-native/ReactCommon/react/renderer/core/tests/RawPropsTest.cpp

@@ -350,7 +350,6 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncomplete) {
   EXPECT_EQ((int)*raw.at("intValue"), 42);
 }
 
-#ifdef REACT_NATIVE_DEBUG
 TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncorrectLookup) {
   ContextContainer contextContainer{};
   PropsParserContext parserContext{-1, contextContainer};
@@ -361,13 +360,9 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncorrectLookup) {
   parser.prepare<PropsPrimitiveTypes>();
   raw.parse(parser);
 
-  // Before D18662135, looking up an invalid key would trigger
-  // an infinite loop. This is out of contract, so we should only
-  // test this in debug.
   EXPECT_EQ(raw.at("flurb"), nullptr);
   EXPECT_EQ((int)*raw.at("intValue"), 42);
 }
-#endif
 
 TEST(RawPropsTest, handlePropsMultiLookup) {
   ContextContainer contextContainer{};