From 8029a546428ef483f6b82a2f6b120c6b8ff2d212 Mon Sep 17 00:00:00 2001
From: Ben Kelly <wanderview@chromium.org>
Date: Tue, 8 Feb 2022 13:32:52 -0800
Subject: [PATCH] URLPattern: Set unmatched optional groups to undefined
 instead of ''.

This addresses the issues raised in:

https://github.com/WICG/urlpattern/issues/162

The main changes in this CL are:

1. The webidl is modified to allow passed back undefined.  I'm told
   real webidl should support `(USVString or undefined)` here, but our
   webidl compiler does not support that.  So we use `any` instead.
2. We must examine if the WTF::String is null or not before populating
   the returned array.  If its null, then we convert to undefined
   instead.

Bug: 1292699
Change-Id: I27a0be2567bb9ce4592ca15a5216fd5a58bdbf17
---
 urlpattern/resources/urlpatterntestdata.json | 30 +++++++++++++-------
 urlpattern/resources/urlpatterntests.js      |  8 ++++++
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/urlpattern/resources/urlpatterntestdata.json b/urlpattern/resources/urlpatterntestdata.json
index 8992c95b59b9c53..6773648da078247 100644
--- a/urlpattern/resources/urlpatterntestdata.json
+++ b/urlpattern/resources/urlpatterntestdata.json
@@ -353,8 +353,9 @@
   {
     "pattern": [{ "pathname": "/foo/:bar?" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "bar": "" } }
+      "pathname": { "input": "/foo", "groups": { "bar": null } }
     }
   },
   {
@@ -418,8 +419,9 @@
   {
     "pattern": [{ "pathname": "/foo/:bar*" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "bar": "" } }
+      "pathname": { "input": "/foo", "groups": { "bar": null } }
     }
   },
   {
@@ -472,15 +474,17 @@
     "expected_obj": {
       "pathname": "/foo/*?"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
     "pattern": [{ "pathname": "/foo/*?" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
@@ -656,15 +660,17 @@
     "expected_obj": {
       "pathname": "/foo/**"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
     "pattern": [{ "pathname": "/foo/**" }],
     "inputs": [{ "pathname": "/foo" }],
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "/foo", "groups": { "0": "" } }
+      "pathname": { "input": "/foo", "groups": { "0": null } }
     }
   },
   {
@@ -1810,9 +1816,10 @@
       "hostname": "(sub.)?example.com",
       "pathname": "/foo"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
       "protocol": { "input": "https", "groups": {} },
-      "hostname": { "input": "example.com", "groups": { "0": "" } },
+      "hostname": { "input": "example.com", "groups": { "0": null } },
       "pathname": { "input": "/foo", "groups": {} }
     }
   },
@@ -1848,9 +1855,10 @@
       "hostname": "(sub(?:.))?example.com",
       "pathname": "/foo"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
       "protocol": { "input": "https", "groups": {} },
-      "hostname": { "input": "example.com", "groups": { "0": "" } },
+      "hostname": { "input": "example.com", "groups": { "0": null } },
       "pathname": { "input": "/foo", "groups": {} }
     }
   },
@@ -2297,9 +2305,10 @@
       "protocol": "data",
       "pathname": "text/javascript,let x = 100/:tens?5;"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
       "protocol": { "input": "data", "groups": {} },
-      "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": "" } }
+      "pathname": { "input": "text/javascript,let x = 100/5;", "groups": { "tens": null } }
     }
   },
   {
@@ -2619,8 +2628,9 @@
     "expected_obj": {
       "pathname": "*(.*)?"
     },
+    "//": "The `null` below is translated to undefined in the test harness.",
     "expected_match": {
-      "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": "" }}
+      "pathname": { "input": "foobar", "groups": { "0": "foobar", "1": null }}
     }
   },
   {
diff --git a/urlpattern/resources/urlpatterntests.js b/urlpattern/resources/urlpatterntests.js
index 40b3edc3da22b99..f774699bff99c3b 100644
--- a/urlpattern/resources/urlpatterntests.js
+++ b/urlpattern/resources/urlpatterntests.js
@@ -146,6 +146,14 @@ function runTests(data) {
             expected_obj.groups['0'] = '';
           }
         }
+        // JSON does not allow us to use undefined directly, so the data file
+        // contains null instead.  Translate to the expected undefined value
+        // here.
+        for (const key in expected_obj.groups) {
+          if (expected_obj.groups[key] === null) {
+            expected_obj.groups[key] = undefined;
+          }
+        }
         assert_object_equals(exec_result[component], expected_obj,
                              `exec() result for ${component}`);
       }