fix(web): decode HTML entities in href so multi-parameter URLs aren't broken

okf/src/reference_agent/web/fetcher.py

@@ -5,13 +5,13 @@
 from urllib.parse import urldefrag, urljoin, urlparse
 from urllib.request import Request, urlopen
 
+from bs4 import BeautifulSoup
 from markdownify import markdownify
 
 _USER_AGENT = "okf-reference-agent/0.1 (+https://github.com/amirhormati/open-knowledge-format)"
 _MAX_MARKDOWN_BYTES = 40 * 1024
 
 _TITLE_RE = re.compile(r"<title[^>]*>(.*?)</title>", re.IGNORECASE | re.DOTALL)
-_HREF_RE = re.compile(r"""href\s*=\s*["']([^"'#\s]+)["']""", re.IGNORECASE)
 
 
 class FetchError(Exception):
@@ -35,10 +35,17 @@ def _extract_title(html: str) -> str | None:
 
 
 def _extract_links(html: str, base_url: str) -> list[str]:
+    # Parse href with an HTML parser rather than a raw-text regex so entities in
+    # attribute values are decoded per the HTML spec. Valid HTML must write `&`
+    # inside an href as `&amp;`, so a regex surfaces `?a=1&amp;b=2` verbatim and
+    # the agent later fetches a literally-broken URL. BeautifulSoup (already a
+    # dependency via markdownify) decodes it correctly — and, unlike a blanket
+    # html.unescape() on the raw string, it leaves query params such as
+    # `&copy=` / `&reg=` intact instead of turning them into `©` / `®`.
     seen: set[str] = set()
     out: list[str] = []
-    for match in _HREF_RE.finditer(html):
-        href = match.group(1).strip()
+    for anchor in BeautifulSoup(html, "html.parser").find_all("a", href=True):
+        href = anchor["href"].strip()
         if not href:
             continue
         scheme = urlparse(href).scheme.lower()

okf/tests/test_web_fetcher.py

@@ -67,3 +67,29 @@ def test_fetch_and_parse_wraps_network_errors():
         with pytest.raises(FetchError) as exc:
             fetch_and_parse("https://example.com/dead")
         assert "connection refused" in str(exc.value)
+
+
+# --- #170: HTML entities in href must be decoded --------------------------
+
+from reference_agent.web.fetcher import _extract_links
+
+
+def test_extract_links_decodes_encoded_ampersands():
+    # Valid HTML encodes `&` as `&`; the extracted URL must use a real `&`.
+    html = '<a href="https://example.com/search?q=foo&amp;lang=en&amp;page=2">x</a>'
+    links = _extract_links(html, "https://example.com/")
+    assert "https://example.com/search?q=foo&lang=en&page=2" in links
+    assert not any("&amp;" in link for link in links)
+
+
+def test_extract_links_preserves_entity_like_query_params():
+    # `&amp;copy=`/`&amp;reg=` must decode to `&copy=`/`&reg=`, never to © / ®.
+    html = '<a href="https://example.com/s?a=1&amp;copy=2&amp;reg=3">x</a>'
+    links = _extract_links(html, "https://example.com/")
+    assert "https://example.com/s?a=1&copy=2&reg=3" in links
+
+
+def test_extract_links_decodes_numeric_entities():
+    html = '<a href="https://example.com/s?q=a&#38;b">x</a>'
+    links = _extract_links(html, "https://example.com/")
+    assert "https://example.com/s?q=a&b" in links