[regexp] Fix the order of named captures on the groups object

schuay · Commit Bot · commit ed40ab15830d · 2019-10-16T07:37:05.000Z
Named capture properties on the groups object should be ordered by the capture index (and not alpha-sorted). This was accidentally broken in https://crrev.com/c/1687413. Bug: v8:9822,v8:9423 Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829 Auto-Submit: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Commit-Queue: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64306}
diff --git a/src/regexp/regexp-ast.h b/src/regexp/regexp-ast.h
@@ -477,7 +477,7 @@ class RegExpCapture final : public RegExpTree {
   int max_match() override { return body_->max_match(); }
   RegExpTree* body() { return body_; }
   void set_body(RegExpTree* body) { body_ = body; }
-  int index() { return index_; }
+  int index() const { return index_; }
   const ZoneVector<uc16>* name() const { return name_; }
   void set_name(const ZoneVector<uc16>* name) { name_ = name; }
   static int StartRegister(int index) { return index * 2; }
diff --git a/src/regexp/regexp-parser.cc b/src/regexp/regexp-parser.cc
@@ -987,18 +987,39 @@ RegExpCapture* RegExpParser::GetCapture(int index) {
   return captures_->at(index - 1);
 }
 
+namespace {
+
+struct RegExpCaptureIndexLess {
+  bool operator()(const RegExpCapture* lhs, const RegExpCapture* rhs) const {
+    DCHECK_NOT_NULL(lhs);
+    DCHECK_NOT_NULL(rhs);
+    return lhs->index() < rhs->index();
+  }
+};
+
+}  // namespace
+
 Handle<FixedArray> RegExpParser::CreateCaptureNameMap() {
   if (named_captures_ == nullptr || named_captures_->empty()) {
     return Handle<FixedArray>();
   }
 
+  // Named captures are sorted by name (because the set is used to ensure
+  // name uniqueness). But the capture name map must to be sorted by index.
+
+  ZoneVector<RegExpCapture*> sorted_named_captures(
+      named_captures_->begin(), named_captures_->end(), zone());
+  std::sort(sorted_named_captures.begin(), sorted_named_captures.end(),
+            RegExpCaptureIndexLess{});
+  DCHECK_EQ(sorted_named_captures.size(), named_captures_->size());
+
   Factory* factory = isolate()->factory();
 
-  int len = static_cast<int>(named_captures_->size()) * 2;
+  int len = static_cast<int>(sorted_named_captures.size()) * 2;
   Handle<FixedArray> array = factory->NewFixedArray(len);
 
   int i = 0;
-  for (const auto& capture : *named_captures_) {
+  for (const auto& capture : sorted_named_captures) {
     Vector<const uc16> capture_name(capture->name()->data(),
                                     capture->name()->size());
     // CSA code in ConstructNewResultFromMatchInfo requires these strings to be
diff --git a/test/mjsunit/harmony/regexp-named-captures.js b/test/mjsunit/harmony/regexp-named-captures.js
@@ -419,6 +419,15 @@ function toSlowMode(re) {
   assertEquals("cd", "abcd".replace(re, "$<$1>"));
 }
 
+// Named captures are ordered by capture index on the groups object.
+// https://crbug.com/v8/9822
+
+{
+  const r = /(?<BKey>.+)\s(?<AKey>.+)/;
+  const s = 'example string';
+  assertArrayEquals(["BKey", "AKey"], Object.keys(r.exec(s).groups));
+}
+
 // Tests for 'groups' semantics on the regexp result object.
 // https://crbug.com/v8/7192
 
