[duplicate] url: fix URLPattern.exec() capture group property insertion order

src/node_url_pattern.cc

@@ -409,22 +409,25 @@ std::optional<ada::url_pattern_init> URLPattern::URLPatternInit::FromJsObject(
 }
 
 MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
-    Environment* env, const ada::url_pattern_component_result& result) {
+    Environment* env,
+    const ada::url_pattern_component_result& result,
+    const std::vector<std::string>& group_name_list) {
   auto isolate = env->isolate();
   auto context = env->context();
   LocalVector<Name> group_names(isolate);
   LocalVector<Value> group_values(isolate);
-  group_names.reserve(result.groups.size());
-  group_values.reserve(result.groups.size());
-  for (const auto& [group_key, group_value] : result.groups) {
+  group_names.reserve(group_name_list.size());
+  group_values.reserve(group_name_list.size());
+  for (const auto& name : group_name_list) {
     Local<Value> key;
-    if (!ToV8Value(context, group_key).ToLocal(&key)) {
+    if (!ToV8Value(context, name).ToLocal(&key)) {
       return {};
     }
     group_names.push_back(key.As<Name>());
     Local<Value> value;
-    if (group_value) {
-      if (!ToV8Value(env->context(), *group_value).ToLocal(&value)) {
+    auto it = result.groups.find(name);
+    if (it != result.groups.end() && it->second) {
+      if (!ToV8Value(env->context(), *it->second).ToLocal(&value)) {
         return {};
       }
     } else {
@@ -457,7 +460,9 @@ MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
 }
 
 MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
-    Environment* env, const ada::url_pattern_result& result) {
+    Environment* env,
+    const ada::url_pattern_result& result,
+    const ada::url_pattern<URLPatternRegexProvider>& url_pattern) {
   auto isolate = env->isolate();
 
   auto tmpl = env->urlpatternresult_template();
@@ -493,14 +498,22 @@ MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
                      return URLPatternInit::ToJsObject(env, init);
                    }
                  }),
-      URLPatternComponentResult::ToJSObject(env, result.protocol),
-      URLPatternComponentResult::ToJSObject(env, result.username),
-      URLPatternComponentResult::ToJSObject(env, result.password),
-      URLPatternComponentResult::ToJSObject(env, result.hostname),
-      URLPatternComponentResult::ToJSObject(env, result.port),
-      URLPatternComponentResult::ToJSObject(env, result.pathname),
-      URLPatternComponentResult::ToJSObject(env, result.search),
-      URLPatternComponentResult::ToJSObject(env, result.hash)};
+      URLPatternComponentResult::ToJSObject(
+          env, result.protocol, url_pattern.protocol_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.username, url_pattern.username_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.password, url_pattern.password_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.hostname, url_pattern.hostname_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.port, url_pattern.port_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.pathname, url_pattern.pathname_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.search, url_pattern.search_component.group_name_list),
+      URLPatternComponentResult::ToJSObject(
+          env, result.hash, url_pattern.hash_component.group_name_list)};
   return NewDictionaryInstanceNullProto(env->context(), tmpl, vals);
 }
 
@@ -552,7 +565,7 @@ MaybeLocal<Value> URLPattern::Exec(Environment* env,
                                    std::optional<std::string_view>& baseURL) {
   if (auto result = url_pattern_.exec(input, baseURL ? &*baseURL : nullptr)) {
     if (result->has_value()) {
-      return URLPatternResult::ToJSValue(env, result->value());
+      return URLPatternResult::ToJSValue(env, result->value(), url_pattern_);
     }
     return Null(env->isolate());
   }

src/node_url_pattern.h

@@ -80,14 +80,18 @@ class URLPattern : public BaseObject {
 
   class URLPatternResult {
    public:
-    static v8::MaybeLocal<v8::Value> ToJSValue(
-        Environment* env, const ada::url_pattern_result& result);
+     static v8::MaybeLocal<v8::Value> ToJSValue(
+         Environment* env,
+         const ada::url_pattern_result& result,
+         const ada::url_pattern<URLPatternRegexProvider>& url_pattern);
   };
 
   class URLPatternComponentResult {
    public:
-    static v8::MaybeLocal<v8::Object> ToJSObject(
-        Environment* env, const ada::url_pattern_component_result& result);
+     static v8::MaybeLocal<v8::Object> ToJSObject(
+         Environment* env,
+         const ada::url_pattern_component_result& result,
+         const std::vector<std::string>& group_name_list);
   };
 
  private:

test/parallel/test-urlpattern-exec-groups-order.js

@@ -0,0 +1,81 @@
+'use strict';
+
+require('../common');
+
+const assert = require('assert');
+const { URLPattern } = require('url');
+
+// Regression test for: URLPattern.exec() does not preserve capture group
+// declaration order. The groups object returned for each component must
+// enumerate its named capture groups in the order they appear in the
+// pattern string.
+
+{
+  // Three-group pathname pattern: one, two, three
+  const pattern = new URLPattern({ pathname: '/:one/:two/:three' });
+  const result = pattern.exec('https://example.com/a/b/c');
+
+  assert.ok(result, 'exec should return a result');
+
+  const keys = Object.keys(result.pathname.groups);
+  assert.deepStrictEqual(
+    keys,
+    ['one', 'two', 'three'],
+    'pathname capture groups must be enumerated in declaration order',
+  );
+  assert.strictEqual(result.pathname.groups.one, 'a');
+  assert.strictEqual(result.pathname.groups.two, 'b');
+  assert.strictEqual(result.pathname.groups.three, 'c');
+}
+
+{
+  // Reversed alphabetical order to distinguish declaration order from sort
+  // order: z comes before a in declaration, so must appear first.
+  const pattern = new URLPattern({ pathname: '/:z/:a/:m' });
+  const result = pattern.exec('https://example.com/1/2/3');
+
+  assert.ok(result, 'exec should return a result');
+
+  assert.deepStrictEqual(
+    Object.keys(result.pathname.groups),
+    ['z', 'a', 'm'],
+    'groups must follow declaration order even when names are not alphabetical',
+  );
+}
+
+{
+  // Multiple components can each carry named groups independently; verify
+  // that declaration order is preserved per-component.
+  const pattern = new URLPattern({
+    hostname: ':sub.:root',
+    pathname: '/:last/:first',
+  });
+  const result = pattern.exec('https://api.example.com/smith/john');
+
+  assert.ok(result, 'exec should return a result');
+
+  assert.deepStrictEqual(
+    Object.keys(result.hostname.groups),
+    ['sub', 'root'],
+    'hostname groups must follow declaration order',
+  );
+
+  assert.deepStrictEqual(
+    Object.keys(result.pathname.groups),
+    ['last', 'first'],
+    'pathname groups must follow declaration order',
+  );
+}
+
+{
+  // A pattern with no named groups should produce an empty groups object.
+  const pattern = new URLPattern({ pathname: '/fixed/path' });
+  const result = pattern.exec('https://example.com/fixed/path');
+
+  assert.ok(result, 'exec should return a result');
+  assert.deepStrictEqual(
+    Object.keys(result.pathname.groups),
+    [],
+    'a pattern without named groups must yield an empty groups object',
+  );
+}