From a72f036bd173fa97ff7734c2d292d7525ff27830 Mon Sep 17 00:00:00 2001
From: Tyler Hallada <tyler@hallada.net>
Date: Sat, 5 Sep 2026 04:42:25 +0000
Subject: [PATCH] Fix a speculation rule that excluded every URL
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The query-string exclusion was written as the pattern string "/*\?*",
meaning "any path, then a literal ?, then anything". It never meant that.
A URL pattern string is split into components before escapes are resolved,
so the ? still ends the pathname: the pattern parses as pathname `/*` with
search `*`, and search `*` matches the empty query too. Sitting inside a
`not`, it excluded every same-origin URL, so both rules reduced to
`(matches) AND NOT (everything)` and no link was ever speculated.

Filter on the query with the component form instead, whose parts are
separate by construction. Name `pathname` alongside `search`: an object
that omits it inherits `/` from the document URL and matches nothing —
the same silent failure wearing different clothes.

Also prerender /issues, which `/issues/*` does not cover.

The MDN example this was modelled on, "/*\?*(^|&)add-to-cart=*", does
work, because what follows the ? lands in the search component and says
something there. Only the bare * collapsed.

Verified by evaluating the shipped JSON through URLPattern: /, /issues,
/issues/<date> and its chapters prerender; ?page=2, /logout, /rate and
/static/* do not; /dashboard* prefetches. Covered by a test that rejects
a ? in any href_matches string and a `search` component without its
`pathname`; it fails on the old rules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0164rQrMUZkf7uV1VGYuCFy4
---
 src/web/mod.rs                  | 74 +++++++++++++++++++++++++++++++++
 src/web/static/speculation.json | 24 +++++++++--
 2 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/src/web/mod.rs b/src/web/mod.rs
index 9daf497..bf11584 100644
--- a/src/web/mod.rs
+++ b/src/web/mod.rs
@@ -1662,6 +1662,80 @@ mod tests {
         assert!(APP_CSS.contains(&format!("url(/static/Newsreader.woff2?v={version})")));
     }
 
+    /// A `\\?` inside an `href_matches` *string* never means what it looks
+    /// like. A URL pattern string is split into components before escapes are
+    /// resolved, so the `?` still ends the pathname: `"/*\\?*"` parses as
+    /// pathname `/*` with search `*`, which matches every same-origin URL.
+    /// Buried in a `not`, that silently excluded everything and no link was
+    /// ever speculated. Filter on the query with the object form instead —
+    /// `{"pathname": "/*", "search": "(.+)"}` — whose components are separate
+    /// by construction.
+    #[test]
+    fn speculation_rules_never_match_the_query_string_from_a_pattern_string() {
+        let rules: serde_json::Value =
+            serde_json::from_str(include_str!("static/speculation.json"))
+                .expect("speculation.json is valid JSON");
+
+        fn walk(value: &serde_json::Value, strings: &mut Vec<String>, objects: &mut usize) {
+            match value {
+                serde_json::Value::Object(map) => {
+                    for (key, child) in map {
+                        if key == "href_matches" {
+                            let patterns = match child {
+                                serde_json::Value::Array(items) => items.clone(),
+                                other => vec![other.clone()],
+                            };
+                            for pattern in patterns {
+                                match pattern {
+                                    serde_json::Value::String(text) => strings.push(text),
+                                    serde_json::Value::Object(components) => {
+                                        assert!(
+                                            !components.contains_key("search")
+                                                || components.contains_key("pathname"),
+                                            "a component pattern naming `search` must also name \
+                                             `pathname`, or the pathname is inherited from the \
+                                             document URL and the pattern matches nothing"
+                                        );
+                                        *objects += 1;
+                                    }
+                                    other => panic!(
+                                        "href_matches takes a string or an object, not {other}"
+                                    ),
+                                }
+                            }
+                        }
+                        walk(child, strings, objects);
+                    }
+                }
+                serde_json::Value::Array(items) => {
+                    for item in items {
+                        walk(item, strings, objects);
+                    }
+                }
+                _ => {}
+            }
+        }
+
+        let mut strings = Vec::new();
+        let mut objects = 0;
+        walk(&rules, &mut strings, &mut objects);
+
+        assert!(
+            !strings.is_empty(),
+            "the rules matched no href_matches at all"
+        );
+        for pattern in &strings {
+            assert!(
+                !pattern.contains('?'),
+                "`{pattern}` reaches for the query from a pattern string; use the object form"
+            );
+        }
+        assert!(
+            objects > 0,
+            "no component pattern is left to exclude query URLs"
+        );
+    }
+
     /// A response that names no policy of its own must not be cacheable: a CDN
     /// cache-everything rule would otherwise give it the CDN's default TTL.
     #[tokio::test]
diff --git a/src/web/static/speculation.json b/src/web/static/speculation.json
index 17b9f59..0e31393 100644
--- a/src/web/static/speculation.json
+++ b/src/web/static/speculation.json
@@ -4,8 +4,17 @@
       "source": "document",
       "where": {
         "and": [
-          { "href_matches": ["/", "/issues/*"] },
-          { "not": { "href_matches": ["/logout", "/rate", "/static/*", "/*\\?*"] } }
+          { "href_matches": ["/", "/issues", "/issues/*"] },
+          {
+            "not": {
+              "href_matches": [
+                "/logout",
+                "/rate",
+                "/static/*",
+                { "pathname": "/*", "search": "(.+)" }
+              ]
+            }
+          }
         ]
       },
       "eagerness": "moderate"
@@ -17,7 +26,16 @@
       "where": {
         "and": [
           { "href_matches": ["/dashboard", "/dashboard/*"] },
-          { "not": { "href_matches": ["/logout", "/rate", "/static/*", "/*\\?*"] } }
+          {
+            "not": {
+              "href_matches": [
+                "/logout",
+                "/rate",
+                "/static/*",
+                { "pathname": "/*", "search": "(.+)" }
+              ]
+            }
+          }
         ]
       },
       "eagerness": "moderate"