Merge pull request #3176 from cloudflare/yagiz/url-parser-href-setter

make sure href setter throws if invalid
cloudflare · Nov 26, 2024 · d9695ca · d9695ca
2 parents 7cae939 + b31604c
commit d9695ca
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 8 deletions.
diff --git a/src/workerd/api/url-standard.c++ b/src/workerd/api/url-standard.c++
@@ -79,8 +79,12 @@ kj::ArrayPtr<const char> URL::getHref() {
   return inner.getHref();
 }
 
-void URL::setHref(kj::String value) {
-  inner.setHref(value);
+void URL::setHref(jsg::Lock& js, kj::String value) {
+  // Href setter is the only place in URL parser that is allowed to throw except the constructor.
+  if (!inner.setHref(value)) {
+    auto context = jsg::TypeErrorContext::setterArgument(typeid(URL), "href");
+    jsg::throwTypeError(js.v8Isolate, context, "valid URL string");
+  }
   KJ_IF_SOME(searchParams, maybeSearchParams) {
     searchParams->reset();
   }

diff --git a/src/workerd/api/url-standard.h b/src/workerd/api/url-standard.h
@@ -162,7 +162,7 @@ class URL: public jsg::Object {
   }
 
   kj::ArrayPtr<const char> getHref();
-  void setHref(kj::String value);
+  void setHref(jsg::Lock& js, kj::String value);
 
   kj::Array<const char> getOrigin();
 

diff --git a/src/workerd/api/wpt/url-test.js b/src/workerd/api/wpt/url-test.js
@@ -8,7 +8,6 @@ export const idnaTestV2Window = run('IdnaTestV2.window.js');
 export const historical = run('historical.any.js', {
   expectedFailures: [
     'Constructor only takes strings',
-    "Setting URL's href attribute and base URLs",
     'URL: no structured serialize/deserialize support',
     'URLSearchParams: no structured serialize/deserialize support',
   ],

diff --git a/src/wpt/harness.js b/src/wpt/harness.js
@@ -50,7 +50,12 @@ globalThis.fetch = async (url) => {
   };
 };
 
-globalThis.GLOBAL = { isWindow: () => false };
+globalThis.self = globalThis;
+globalThis.GLOBAL = {
+  isWindow() {
+    return false;
+  },
+};
 
 globalThis.done = () => undefined;
 
@@ -303,16 +308,22 @@ function sanitizeMessage(message) {
   );
 }
 
-async function validate(options) {
+function validate(testFileName, options) {
   const expectedFailures = new Set(options.expectedFailures ?? []);
 
+  let failing = false;
   for (const err of globalThis.errors) {
     if (!expectedFailures.delete(err.message)) {
       err.message = sanitizeMessage(err.message);
-      throw err;
+      console.error(err);
+      failing = true;
     }
   }
 
+  if (failing) {
+    throw new Error(`${testFileName} failed`);
+  }
+
   if (expectedFailures.size > 0) {
     console.info('Expected failures', expectedFailures);
     throw new Error(
@@ -326,7 +337,7 @@ export function run(file, options = {}) {
     async test() {
       prepare(options);
       await import(file);
-      await validate(options);
+      validate(file, options);
     },
   };
 }