fix: When converting a URL to a string, do not add a ? if there are no query string parameters

fixes #484

Author: Jake Champion

.github/workflows/main.yml

@@ -239,6 +239,7 @@ jobs:
           - streaming-close
           - tee
           - timers
+          - urlsearchparams
     steps:
     - name: Checkout fastly/js-compute-runtime
       uses: actions/checkout@v3
@@ -378,6 +379,7 @@ jobs:
           - 'secret-store'
           - 'status'
           - 'timers'
+          - 'urlsearchparams'
     steps:
     - uses: actions/checkout@v3
       with:

c-dependencies/js-compute-runtime/rust-url/src/lib.rs

@@ -36,13 +36,23 @@ impl JSUrlSearchParams {
         match self.url_or_str {
             UrlOrString::Url(url) => {
                 let url = unsafe { url.as_mut().unwrap() };
-                let mut pairs = url.url.query_pairs_mut();
-                pairs.clear().extend_pairs(self.list.iter());
+                if self.list.is_empty() {
+                    url.url.set_query(None);
+                } else {
+                    let mut pairs = url.url.query_pairs_mut();
+                    pairs.clear();
+                    pairs.extend_pairs(self.list.iter());
+                }
             }
             UrlOrString::Str(_) => {
-                let str = form_urlencoded::Serializer::new(String::new())
-                    .extend_pairs(&*self.list)
-                    .finish();
+                let str;
+                if self.list.is_empty() {
+                    str = form_urlencoded::Serializer::new(String::new()).finish();
+                } else {
+                    str = form_urlencoded::Serializer::new(String::new())
+                        .extend_pairs(&*self.list)
+                        .finish();
+                }
                 self.url_or_str = UrlOrString::Str(str);
             }
         }

integration-tests/js-compute/fixtures/urlsearchparams/bin/index.js

@@ -0,0 +1,41 @@
+/* eslint-env serviceworker */
+/* global ReadableStream ObjectStore ObjectStoreEntry */
+import { env } from 'fastly:env';
+import { pass, fail, assert, assertThrows, assertRejects, assertResolves } from "../../../assertions.js";
+
+addEventListener("fetch", event => {
+    event.respondWith(app(event))
+})
+/**
+ * @param {FetchEvent} event
+ * @returns {Response}
+ */
+async function app(event) {
+    try {
+        const path = (new URL(event.request.url)).pathname;
+        console.log(`path: ${path}`)
+        console.log(`FASTLY_SERVICE_VERSION: ${env('FASTLY_SERVICE_VERSION')}`)
+        if (routes.has(path)) {
+            const routeHandler = routes.get(path);
+            return await routeHandler()
+        }
+        return fail(`${path} endpoint does not exist`)
+    } catch (error) {
+        return fail(`The routeHandler threw an error: ${error.message}` + '\n' + error.stack)
+    }
+}
+
+const routes = new Map();
+routes.set('/', () => {
+    routes.delete('/');
+    let test_routes = Array.from(routes.keys())
+    return new Response(JSON.stringify(test_routes), { 'headers': { 'content-type': 'application/json' } });
+});
+
+routes.set("/urlsearchparams/sort", async () => {
+    const urlObj = new URL('http://www.example.com');
+    urlObj.searchParams.sort();
+    let error = assert(urlObj.toString(), 'http://www.example.com/', `urlObj.toString()`)
+    if (error) { return error }
+    return pass()
+});

integration-tests/js-compute/fixtures/urlsearchparams/fastly.toml.in

@@ -0,0 +1,12 @@
+# This file describes a Fastly Compute@Edge package. To learn more visit:
+# https://developer.fastly.com/reference/fastly-toml/
+
+authors = ["[email protected]"]
+description = ""
+language = "other"
+manifest_version = 2
+name = "urlsearchparams"
+service_id = ""
+
+[scripts]
+  build = "node ../../../../js-compute-runtime-cli.js"

integration-tests/js-compute/fixtures/urlsearchparams/tests.json

@@ -0,0 +1,12 @@
+{
+  "GET /urlsearchparams/sort": {
+    "environments": ["c@e", "viceroy"],
+    "downstream_request": {
+      "method": "GET",
+      "pathname": "/urlsearchparams/sort"
+    },
+    "downstream_response": {
+      "status": 200
+    }
+  }
+}

tests/wpt-harness/expectations/url/urlsearchparams-delete.any.js.json

@@ -6,9 +6,9 @@
     "status": "PASS"
   },
   "Deleting all params removes ? from URL": {
-    "status": "FAIL"
+    "status": "PASS"
   },
   "Removing non-existent param removes ? from URL": {
-    "status": "FAIL"
+    "status": "PASS"
   }
 }

tests/wpt-harness/expectations/url/urlsearchparams-sort.any.js.json

@@ -48,6 +48,6 @@
     "status": "PASS"
   },
   "Sorting non-existent params removes ? from URL": {
-    "status": "FAIL"
+    "status": "PASS"
   }
 }