Skip to content

URLSource caches raw page HTML: scripts, comments and attributes reach a committed cache #92

Description

@cmungall

URLSource caches the raw HTML it fetched, including <script> blocks, inline styles, comments, and every tag attribute. For a project that commits its reference cache to a public git repository, that stores a lot of material that is not the reference text — and page scripts and data attributes routinely carry things that should not be copied into somebody's repo, including signed asset URLs and API-key parameters.

In the dismech cache, 150 of 199 content_type: url entries contain a <script> block.

The exposure there turns out to be benign on inspection — the signed URLs are public CDN figure links with Expires=2147483647, and the one api_key is an empty template parameter — so this is a hygiene issue rather than an incident. But the general shape is not benign: whatever a publisher puts in a page script ends up verbatim in a committed cache file, and nobody reviews 199 HTML dumps.

Unlike the HTML full text path, which goes through HTMLExtractor, URLSource does no extraction at all — it stores the response body. I checked 0.3.0rc1: url.py has no script removal, no comment stripping, and no attribute filtering.

Suggested fix

Sanitize before caching, keeping what a quoted excerpt can legitimately need:

  • drop script, style, noscript, template elements and HTML comments;
  • drop all tag attributes except table-structural ones (rowspan, colspan, scope), which carry meaning for a quoted table row;
  • leave plain text, XML, and extracted PDF content alone.

This is source extraction rather than a rendering decision, so it does not need to model what a browser would show. Keeping body markup and table structure is enough for excerpt validation.

Context

dismech currently does this as a runtime patch over URLSource, which is how I found it. That patch is the last one it still carries — the other ten were retired once their fixes landed here (#66-74, #85, #87, #88) — and it will be deleted when this lands. Filing it rather than keeping the workaround quiet, per the rule that a patch needs an upstream issue and an exit.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions