Skip to content

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

Open
vinodborole wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
vinodborole:fix/decode-href-entities
Open

fix(web): decode HTML entities in href so multi-parameter URLs aren't broken#174
vinodborole wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
vinodborole:fix/decode-href-entities

Conversation

@vinodborole

Copy link
Copy Markdown

Fixes #170.

_extract_links scraped href values out of raw HTML with a regex, so HTML entities in attribute values were never decoded. Because valid HTML requires & inside an href to be written as &, a link like

<a href="https://example.com/search?q=foo&amp;lang=en">

was surfaced verbatim as ...?q=foo&amp;lang=en, and fetch_url then requested a URL with a literal &amp; in the query string — which servers reject or misparse. This hits any correctly-encoded page (Google/GitHub search results, most CMS output).

Fix: parse href with BeautifulSoup — already a dependency via markdownify — instead of a raw-text regex. The HTML parser decodes attribute entities per the spec.

Notably, the tempting one-liner (html.unescape(href)) is not correct here: it uses HTML's text rules and over-decodes query params that match no-semicolon legacy entities, turning ?a=1&copy=2&reg=3 into ?a=1©=2®=3. Parsing the attribute avoids that — &amp;copy= decodes to &copy=, not ©. A regression test guards this.

Also removes the now-unused _HREF_RE, and as a side benefit the parser handles hrefs the regex missed (across newlines, unquoted, or after other attributes).

Tests

All green (PYTHONPATH=src pytest okf/tests/test_web_fetcher.py, 7 passed):

  • the multi-parameter repro from the issue,
  • an entity-like-param guard (&copy= / &reg= stay literal, not © / ®),
  • a numeric-entity case (&#38;).

Known limitation: genuinely malformed HTML with a bare &copy= (invalid — should be &amp;copy=) remains ambiguous; matching a browser's full "ambiguous ampersand" tokenizer rule would need a stricter parser. Out of scope for the reported (spec-conformant) case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spec-conformant HTML pages with properly-encoded multi-parameter URLs hand the agent a broken URL

1 participant