Add fetch_standard_url compat flag

src/workerd/api/http-standard-test.js

@@ -0,0 +1,27 @@
+import { strictEqual } from 'node:assert';
+
+export default {
+  async fetch(req) {
+    if (req.url.endsWith('/')) {
+      return new Response(null, {
+        status: 302,
+        headers: {
+          location: ' /\t2  '
+        }
+      });
+    } else if (req.url.endsWith('/2')) {
+      return new Response("ok");
+    }
+  }
+};
+
+export const test = {
+  async test(ctrl, env) {
+
+    const resp = await env.SERVICE.fetch(' http://p\tl\na\tc\ne\th\no\tl\nd\te\nr\t/ ');
+
+    console.log(resp.url);
+
+    strictEqual(await resp.text(), "ok");
+  }
+};

src/workerd/api/http-standard-test.wd-test

@@ -0,0 +1,18 @@
+using Workerd = import "/workerd/workerd.capnp";
+
+const unitTests :Workerd.Config = (
+  services = [
+    ( name = "http-standard-test",
+      worker = (
+        modules = [
+          ( name = "worker", esModule = embed "http-standard-test.js" )
+        ],
+        bindings = [
+          ( name = "SERVICE", service = "http-standard-test" )
+        ],
+        compatibilityDate = "2023-08-01",
+        compatibilityFlags = ["nodejs_compat", "fetch_standard_url"],
+      )
+    ),
+  ],
+);

src/workerd/api/http.c++

@@ -906,6 +906,22 @@ jsg::Ref<Request> Request::constructor(
   KJ_SWITCH_ONEOF(input) {
     KJ_CASE_ONEOF(u, kj::String) {
       url = kj::mv(u);
+
+      // TODO(later): This is rather unfortunate. The original implementation of
+      // this used non-standard URL parsing in violation of the spec. Unfortunately
+      // some users have come to depend on the non-standard behavior so we have to
+      // gate the standard behavior with a compat flag. Ideally we'd just be able to
+      // use the standard parsed URL throughout all of the code but in order to
+      // minimize the number of changes, we're going to ultimately end up double
+      // parsing (and serializing) the URL... here we parse it with the standard
+      // parser, reserialize it back into a string for the sake of not modifying
+      // the rest of the implementation. Fortunately the standard parser is fast
+      // but it would eventually be nice to eliminate the double parsing.
+      if (FeatureFlags::get(js).getFetchStandardUrl()) {
+        auto parsed = JSG_REQUIRE_NONNULL(jsg::Url::tryParse(url.asPtr()), TypeError,
+            kj::str("Invalid URL: ", url));
+        url = kj::str(parsed.getHref());
+      }
     }
     KJ_CASE_ONEOF(r, jsg::Ref<Request>) {
       // Check to see if we're getting a new body from `init`. If so, we want to ignore `input`'s
@@ -1929,7 +1945,29 @@ jsg::Promise<jsg::Ref<Response>> handleHttpRedirectResponse(
   //   - Translates HEAD requests that hit 303 into HEAD requests with null bodies.
   //   - Translates all other requests that hit 303 into GET requests with null bodies.
 
-  auto redirectedLocation = urlList.back().tryParseRelative(location);
+  auto redirectedLocation = ([&]() -> kj::Maybe<kj::Url> {
+    // TODO(later): This is a bit unfortunate. Per the fetch spec, we're supposed to be
+    // using standard WHATWG URL parsing to resolve the redirect URL. However, changing it
+    // now requires a compat flag. In order to minimize changes to the rest of the impl
+    // we end up double parsing the URL here, once with the standard parser to produce the
+    // correct result, and again with kj::Url in order to produce something that works with
+    // the existing code. Fortunately the standard parser is fast but it would be nice to
+    // be able to avoid the double parse at some point.
+    if (FeatureFlags::get(js).getFetchStandardUrl()) {
+      auto base = urlList.back().toString();
+      KJ_IF_SOME(parsed, jsg::Url::tryParse(location, base.asPtr())) {
+        auto str = kj::str(parsed.getHref());
+        return kj::Url::tryParse(str.asPtr(), kj::Url::Context::REMOTE_HREF, kj::Url::Options {
+          .allowEmpty = true,
+          .percentDecode = false,
+        });
+      } else {
+        return kj::none;
+      }
+    } else {
+      return urlList.back().tryParseRelative(location);
+    }
+  })();
 
   if (redirectedLocation == kj::none) {
     auto exception = JSG_KJ_EXCEPTION(FAILED, TypeError,

src/workerd/io/compatibility-date.capnp

@@ -435,4 +435,10 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
   # from the Request or Response body is not compliant with the standard. Unfortunately,
   # making it compliant is a breaking change. This flag controls the availability of the
   # new spec-compliant Blob mime type normalization.
+
+  fetchStandardUrl @49 :Bool
+    $compatEnableFlag("fetch_standard_url")
+    $compatDisableFlag("fetch_legacy_url")
+    $compatEnableDate("2024-06-03");
+  # Ensures that WHATWG standard URL parsing is used in the fetch API implementation.
 }