fix(lab-ops-hub): return empty path, userinfo and ?/# markers WHATWG-normalized (#639)

pkgs/lab-ops-hub/container/app.py

@@ -400,7 +400,7 @@ def validate_crawl_url(raw):
             # The returned form is WHATWG-normalized (#633).
             return _url_with_encoded_components(
                 _normalize_validated_url(
-                    _url_with_host(u, host) if percent_decoded else raw
+                    _url_with_host(u, host, raw) if percent_decoded else raw
                 )
             )
     embedded = _embedded_ipv4(ip)
@@ -453,7 +453,7 @@ def validate_crawl_url(raw):
         _canonical_ipv6(ip) if isinstance(ip, ipaddress.IPv6Address) else str(ip)
     )
     return _url_with_encoded_components(
-        _normalize_validated_url(_url_with_host(u, canonical))
+        _normalize_validated_url(_url_with_host(u, canonical, raw))
     )
 
 
@@ -716,11 +716,16 @@ def _canonical_ipv6(ip):
     return f"{head}::{tail}"
 
 
-def _url_with_host(u, new_host):
+def _url_with_host(u, new_host, source_url=None):
     # Rebuild a parsed URL with new_host in place of the netloc's host,
     # preserving userinfo, port, path, query and fragment. Used for
     # percent-decoded hosts (#529): the fetcher must connect to the
-    # decoded host that was judged, not the raw spelling.
+    # decoded host that was judged, not the raw spelling. The parsed
+    # parts cannot represent an empty "?" / "#" marker (#639), so when
+    # the caller passes the source spelling its markers are re-attached
+    # here — otherwise this geturl() rebuild would drop them before
+    # the normalizer downstream ever sees them.
+    markers = _empty_query_fragment_markers(source_url) if source_url else ""
     netloc = u.netloc
     if "@" in netloc:
         userinfo, _, hostport = netloc.rpartition("@")
@@ -729,11 +734,11 @@ def _url_with_host(u, new_host):
         prefix, hostport = "", netloc
     if hostport.startswith("["):
         _, _, suffix = hostport.partition("]")
-        return u._replace(netloc=f"{prefix}[{new_host}]{suffix}").geturl()
+        return u._replace(netloc=f"{prefix}[{new_host}]{suffix}").geturl() + markers
     if ":" in hostport:
         _, _, port = hostport.partition(":")
-        return u._replace(netloc=f"{prefix}{new_host}:{port}").geturl()
-    return u._replace(netloc=prefix + new_host).geturl()
+        return u._replace(netloc=f"{prefix}{new_host}:{port}").geturl() + markers
+    return u._replace(netloc=prefix + new_host).geturl() + markers
 
 
 _ASCII_LOWER = str.maketrans("ABCDEFGHIJKLMNOPQRSTUVWXYZ", "abcdefghijklmnopqrstuvwxyz")
@@ -776,13 +781,36 @@ def _resolve_dot_segments(path):
     return "/".join(out)
 
 
+def _empty_query_fragment_markers(url):
+    # urlparse cannot represent an empty query or fragment: it parses
+    # "http://example.com/path?" and "http://example.com/path" to the
+    # same parts, and geturl() drops the marker. WHATWG keeps it — the
+    # query/fragment is present but empty — and the Worker returns it
+    # (#639), so the markers are computed from the spelling as a suffix
+    # and re-attached after each geturl() rebuild. A "?" after the "#"
+    # is fragment data, not a query marker, and a marker is only owed
+    # when the parsed component is empty (a non-empty one is rebuilt
+    # by geturl() itself).
+    u = urlparse(url)
+    markers = ""
+    if not u.query and "?" in url.partition("#")[0]:
+        markers += "?"
+    if not u.fragment and "#" in url:
+        markers += "#"
+    return markers
+
+
 def _normalize_validated_url(url):
     # Return a validated URL in the WHATWG-normalized form the Worker
     # returns (#633), so both crawlers record — and fetch — the same
     # URL for the same target: dot segments in the path resolved,
     # the host lowercased, and the port canonicalized (the scheme's
     # default port dropped, leading zeros on any other port stripped,
-    # an empty port's colon dropped). Only ASCII case folds in the
+    # an empty port's colon dropped). The empty components are
+    # normalized the same way (#639): an empty path becomes "/", an
+    # empty userinfo's "@" is dropped, and empty "?" / "#" markers
+    # are kept (the decision on those markers: preserve them, matching
+    # the Worker byte for byte). Only ASCII case folds in the
     # host: the IDN spelling itself is deliberately kept, as before
     # (judged in mapped form, fetched via IDNA — #615); urlparse has
     # already lowercased the scheme. The path's ";params" tail is
@@ -793,7 +821,7 @@ def _normalize_validated_url(url):
     netloc = u.netloc
     if "@" in netloc:
         userinfo, _, hostport = netloc.rpartition("@")
-        prefix = userinfo + "@"
+        prefix = userinfo + "@" if userinfo else ""
     else:
         prefix, hostport = "", netloc
     if hostport.startswith("["):
@@ -819,11 +847,17 @@ def _normalize_validated_url(url):
         else:
             port_norm = f":{port_span}"
     full_path = u.path + (f";{u.params}" if u.params else "")
+    resolved_path = _resolve_dot_segments(full_path)
+    if not resolved_path:
+        # A special-scheme URL with an empty path serializes with
+        # the path "/" in WHATWG ("http://example.com" is
+        # "http://example.com/") (#639).
+        resolved_path = "/"
     return u._replace(
         netloc=f"{prefix}{host_norm}{port_norm}",
-        path=_resolve_dot_segments(full_path),
+        path=resolved_path,
         params="",
-    ).geturl()
+    ).geturl() + _empty_query_fragment_markers(url)
 
 
 # WHATWG's per-component percent-encode sets for the ASCII punctuation
@@ -889,14 +923,18 @@ def _url_with_encoded_components(url):
     netloc = u.netloc
     if "@" in netloc:
         userinfo, _, hostport = netloc.rpartition("@")
-        netloc = f"{_encode_userinfo(userinfo)}@{hostport}"
+        # An empty userinfo serializes away in WHATWG — the "@" is
+        # dropped with it (#639); _normalize_validated_url already
+        # drops it on the validate path, this keeps the helper
+        # consistent when called on its own.
+        netloc = f"{_encode_userinfo(userinfo)}@{hostport}" if userinfo else hostport
     return u._replace(
         netloc=netloc,
         path=_encode_url_component(u.path, _PATH_EXTRA),
         params=_encode_url_component(u.params, _PATH_EXTRA),
         query=_encode_url_component(u.query, _QUERY_EXTRA),
         fragment=_encode_url_component(u.fragment, _FRAGMENT_EXTRA),
-    ).geturl()
+    ).geturl() + _empty_query_fragment_markers(url)
 
 
 # Backwards-compatible alias: the #615 name for the same helper.

pkgs/lab-ops-hub/test/lib.test.ts

@@ -80,6 +80,30 @@ test("validateCrawlUrl returns dot segments, host case and ports normalized (#63
   }
 });
 
+test("validateCrawlUrl returns empty components normalized (#639)", () => {
+  // The Worker's returned form is the reference the container validator
+  // mirrors: "/" supplied for an empty path, the empty port's colon and
+  // the empty userinfo's "@" dropped, and empty "?" / "#" markers kept
+  // (the query/fragment is present but empty).
+  const cases: Array<[string, string]> = [
+    ["http://example.com", "http://example.com/"],
+    ["https://example.com", "https://example.com/"],
+    ["http://example.com:8080", "http://example.com:8080/"],
+    ["http://example.com:/path", "http://example.com/path"],
+    ["http://@example.com/", "http://example.com/"],
+    ["http://example.com/path?", "http://example.com/path?"],
+    ["http://example.com/path#", "http://example.com/path#"],
+    ["http://example.com?", "http://example.com/?"],
+    ["http://example.com/path?#", "http://example.com/path?#"],
+    ["http://user@example.com", "http://user@example.com/"],
+  ];
+  for (const [raw, expected] of cases) {
+    const r = validateCrawlUrl(raw);
+    assert.equal(r.ok, true, raw);
+    if (r.ok) assert.equal(r.url, expected, raw);
+  }
+});
+
 test("validateCrawlUrl refuses non-http schemes and junk", () => {
   assert.equal(validateCrawlUrl("ftp://example.com/").ok, false);
   assert.equal(validateCrawlUrl("file:///etc/passwd").ok, false);

pkgs/lab-ops-hub/test/test_container_validate.py

@@ -1002,6 +1002,67 @@ def test_redirect_handler_follows_the_normalized_target(self):
         )
         self.assertEqual(followed.full_url, "http://example.org/b")
 
+    def test_returns_empty_components_normalized(self):
+        # The container returned an empty path, an empty port and an
+        # empty userinfo verbatim while the Worker returns the
+        # WHATWG-normalized form, so the two crawlers recorded
+        # different URLs for the same target (#639). The returned URL
+        # now supplies "/" for an empty path and drops the empty
+        # userinfo's "@" (the empty port's colon is dropped by the
+        # #633 port canonicalization). Decision on the empty "?" / "#"
+        # markers urlparse cannot represent: preserve them, matching
+        # the Worker byte for byte — the query/fragment is present
+        # but empty. Verdicts are unchanged.
+        cases = {
+            "http://example.com": "http://example.com/",
+            "https://example.com": "https://example.com/",
+            "http://example.com:8080": "http://example.com:8080/",
+            "http://example.com:80": "http://example.com/",
+            "http://example.com:/path": "http://example.com/path",
+            "http://@example.com/": "http://example.com/",
+            "http://@example.com": "http://example.com/",
+            "http://@example.com/path?x#": "http://example.com/path?x#",
+            "http://example.com/path?": "http://example.com/path?",
+            "http://example.com/path#": "http://example.com/path#",
+            "http://example.com?": "http://example.com/?",
+            "http://example.com/path?#": "http://example.com/path?#",
+            "http://example.com#frag?notquery": ("http://example.com/#frag?notquery"),
+            "http://8.8.8.8": "http://8.8.8.8/",
+            "http://134744072": "http://8.8.8.8/",
+            "http://[2001:4860:4860::8888]": "http://[2001:4860:4860::8888]/",
+            "http://user@example.com": "http://user@example.com/",
+            "http://example.com/a/../b": "http://example.com/b",
+            # The markers survive the host-substitution rebuilds too:
+            # percent-decoded hosts and canonical IP literals are
+            # rebuilt with geturl() before normalization (#639).
+            "http://%65xample.com/path?": "http://example.com/path?",
+            "http://%65xample.com?": "http://example.com/?",
+            "http://8%2e8%2e8%2e8/path#": "http://8.8.8.8/path#",
+            "http://8%2e8%2e8%2e8?#": "http://8.8.8.8/?#",
+        }
+        for raw, expected in cases.items():
+            self.assertEqual(validate_crawl_url(raw), expected, raw)
+        # Non-empty userinfo, query and fragment pass through
+        # byte-identical; a "?" inside the query or fragment is data.
+        self.assertAccepted("http://user:pass@example.com/path?q=1#frag")
+        self.assertAccepted("http://example.com/pa?th#fr?ag")
+        # Blocked targets in these spellings are refused by the
+        # ordinary checks, for the right reason.
+        self.assertRefused("http://@127.0.0.1/")
+        self.assertRefused("http://127.0.0.1")
+        self.assertRefused("http://@localhost/")
+        # Redirect targets inherit the normalization (#639): the
+        # handler validates the raw Location and fetches the form
+        # validate_crawl_url returns.
+        import urllib.request
+
+        handler = app._ValidatingRedirectHandler()
+        req = urllib.request.Request("http://example.com/")
+        followed = handler.redirect_request(
+            req, None, 302, "Found", {}, "http://EXAMPLE.org"
+        )
+        self.assertEqual(followed.full_url, "http://example.org/")
+
     def test_percent_encodes_non_ascii_outside_the_host(self):
         # WHATWG percent-encodes non-ASCII characters in the userinfo,
         # path, query and fragment as UTF-8; urlparse hands them through