Text.
Text.
x
", "html.parser")) is None + >>> find_pmc_article_body(BeautifulSoup("x
", "html.parser")) is None True """ for class_name in PMC_ARTICLE_BODY_CLASSES: @@ -192,7 +198,7 @@ def _fetch_pmc_html(self, pmcid: str, config: ReferenceValidationConfig) -> Opti ) soup = BeautifulSoup(response.content, "html.parser") - article_body = _find_pmc_article_body(soup) + article_body = find_pmc_article_body(soup) if article_body: # The region is selected here, but the text comes out of the shared # extractor rather than a private copy of the paragraph walk, so diff --git a/src/linkml_reference_validator/etl/reference_fetcher.py b/src/linkml_reference_validator/etl/reference_fetcher.py index 5724d7d..4e42527 100644 --- a/src/linkml_reference_validator/etl/reference_fetcher.py +++ b/src/linkml_reference_validator/etl/reference_fetcher.py @@ -521,9 +521,12 @@ def _preserve_cached_full_text( cached.content_type, ) - # May its text be served? Asked without the bypass, so stale HTML is - # withheld here exactly as _stale_fallback withholds it, and validation - # falls back to the freshly fetched abstract. + # May its text be served? Asked with the consumer's serve policy, the + # same one _stale_fallback uses. By default stale HTML is withheld and + # validation falls back to the freshly fetched abstract; a consumer that + # has opted in gets the cached body instead. This is the path a refresh + # normally takes -- the source still answers, with an abstract -- so it + # is the one most consumers see, not _stale_fallback. servable = self._load_from_disk( normalized_reference_id, allow_stale=True, @@ -1536,8 +1539,11 @@ def _load_from_disk( ): logger.warning( "Refusing stale HTML full text for %s: it may be a repository " - "landing page. Retry when the source serves full text again to " - "repair it.", + "landing page. A re-fetch repairs it where the source still " + "serves full text -- but not for a PMC-only article, which " + "answers a Python HTTP client with a bot-check interstitial on " + "an HTTP 200. Set serve_stale_html_full_text if this cache is " + "one you review.", reference_id, ) return None diff --git a/src/linkml_reference_validator/etl/sources/pmid.py b/src/linkml_reference_validator/etl/sources/pmid.py index d2439a3..fff79e4 100644 --- a/src/linkml_reference_validator/etl/sources/pmid.py +++ b/src/linkml_reference_validator/etl/sources/pmid.py @@ -23,6 +23,7 @@ from bs4 import BeautifulSoup # type: ignore import requests # type: ignore +from linkml_reference_validator.etl.fulltext.pmc import find_pmc_article_body from linkml_reference_validator.models import ReferenceContent, ReferenceValidationConfig from linkml_reference_validator.etl.extract.html import HTMLExtractor from linkml_reference_validator.etl.extract import MIN_FULLTEXT_CHARS @@ -582,12 +583,7 @@ def _fetch_pmc_html( return None soup = BeautifulSoup(response.content, "html.parser") - # Shared with the PMC full-text provider: the container moved from a - # ``div`` to a ``section`` and changed class, so both call sites went - # blind at once (dismech#12672). - from linkml_reference_validator.etl.fulltext.pmc import _find_pmc_article_body - - article_body = _find_pmc_article_body(soup) + article_body = find_pmc_article_body(soup) if article_body: # Region selected here, text extracted by the shared extractor, for diff --git a/src/linkml_reference_validator/etl/sources/url.py b/src/linkml_reference_validator/etl/sources/url.py index 384e945..00fa6c3 100644 --- a/src/linkml_reference_validator/etl/sources/url.py +++ b/src/linkml_reference_validator/etl/sources/url.py @@ -92,6 +92,19 @@ def fetch( ) content = self._decode(data, content_type_header) + + if self._is_bot_challenge(content): + # An HTTP 200 carrying a challenge page. Nothing about the reference + # is known, so this is a transient failure, not an absence: caching + # it would record an interstitial's" + ("Real article prose. " * 40) + "
" @@ -34,7 +34,7 @@ ) def test_article_containers_are_found(markup, label): soup = BeautifulSoup(markup, "html.parser") - assert _find_pmc_article_body(soup) is not None, f"{label} should be recognised" + assert find_pmc_article_body(soup) is not None, f"{label} should be recognised" @pytest.mark.parametrize( @@ -49,11 +49,27 @@ def test_article_containers_are_found(markup, label): ) def test_pages_without_an_article_container_are_declined(markup, label): soup = BeautifulSoup(markup, "html.parser") - assert _find_pmc_article_body(soup) is None, f"{label} must not be accepted" + assert find_pmc_article_body(soup) is None, f"{label} must not be accepted" def test_a_bare_body_element_is_not_an_article_container(): """```` is on every page, so matching class="body" alone would accept an interstitial. Only the article-body classes count.""" soup = BeautifulSoup(f"{BODY}", "html.parser") - assert _find_pmc_article_body(soup) is None + assert find_pmc_article_body(soup) is None + + +def test_a_challenge_page_carrying_a_classed_body_is_still_declined(): + """The interstitial arrives on an HTTP 200, so this selector is the only check. + + PMC's challenge page carries a classed ````. None of its classes may + overlap the article-body list, or a bot-check page gets cached as full text. + """ + markup = ( + '' + "Enable JavaScript and cookies to continue.
" + "" + ) + soup = BeautifulSoup(markup, "html.parser") + assert find_pmc_article_body(soup) is None diff --git a/tests/test_serve_stale_html_optin.py b/tests/test_serve_stale_html_optin.py index d0a587d..7c7dadc 100644 --- a/tests/test_serve_stale_html_optin.py +++ b/tests/test_serve_stale_html_optin.py @@ -22,6 +22,7 @@ from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher from linkml_reference_validator.models import ( + ReferenceContent, ReferenceValidationConfig, ) @@ -73,3 +74,61 @@ def test_opting_in_does_not_stamp_or_rewrite_the_entry(tmp_path): text = (tmp_path / "PMID_30929739.md").read_text(encoding="utf-8") assert "extractor_version" not in text assert BODY_SENTENCE in text + + +def _fetch_with_abstract_source(tmp_path, **cfg): + """The path a refresh normally takes: the source answers, with an abstract. + + ``_stale_fallback`` needs the source to yield *nothing*. For a PMC-only + article PubMed still returns the abstract, so the refresh reaches + ``_preserve_cached_full_text`` instead: the overwrite is refused, and then a + separate question decides what may be served. That second question is the + one the flag has to answer, and it is the one dismech actually hits. + """ + _cache(tmp_path) + fetcher = ReferenceFetcher( + ReferenceValidationConfig( + cache_dir=tmp_path, email="me@example.org", fetch_full_text=False, **cfg + ) + ) + fresh = ReferenceContent( + reference_id="PMID:30929739", + content="Just the abstract.", + content_type="abstract_only", + title="A paper", + ) + + class _Source: + def fetch(self, identifier, config): + return fresh + + with patch( + "linkml_reference_validator.etl.reference_fetcher" + ".ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + return fetcher.fetch("PMID:30929739") + + +def test_refused_overwrite_serves_the_abstract_by_default(tmp_path): + content = _fetch_with_abstract_source(tmp_path) + assert content is not None + assert BODY_SENTENCE not in (content.content or ""), ( + "by default the cached body stays withheld even though it was preserved" + ) + assert "Just the abstract." in (content.content or "") + + +def test_refused_overwrite_serves_the_cached_body_when_opted_in(tmp_path): + content = _fetch_with_abstract_source(tmp_path, serve_stale_html_full_text=True) + assert content is not None + assert BODY_SENTENCE in (content.content or "") + + +def test_refused_overwrite_leaves_the_file_alone_either_way(tmp_path): + """Serving is not overwriting: the entry keeps its body and stays un-stamped.""" + for flag in (False, True): + _fetch_with_abstract_source(tmp_path, serve_stale_html_full_text=flag) + text = (tmp_path / "PMID_30929739.md").read_text(encoding="utf-8") + assert BODY_SENTENCE in text, f"body lost with flag={flag}" + assert "extractor_version" not in text, f"entry stamped with flag={flag}" diff --git a/tests/test_url_bot_challenge.py b/tests/test_url_bot_challenge.py new file mode 100644 index 0000000..be00902 --- /dev/null +++ b/tests/test_url_bot_challenge.py @@ -0,0 +1,74 @@ +"""A bot-check page is not a reference, and must not become one. + +PMC answers a Python HTTP client with a challenge page carried on an HTTP 200, +so no status check sees it. ``URLSource`` took its ``Enable JavaScript and cookies to continue.
", + id="pmc-checking-your-browser", + ), + pytest.param( + "We have detected unusual traffic from your network.
", + id="unusual-traffic", + ), +] + + +@pytest.mark.parametrize("html", CHALLENGE_PAGES) +def test_a_challenge_page_is_recognised(html): + assert URLSource._is_bot_challenge(html) is True + + +@pytest.mark.parametrize( + "html", + [ + pytest.param( + "" + + ("Real article prose. " * 40) + + "
" + ("Prose discussing CAPTCHA research. " * 30) + "
", + id="article-whose-subject-is-bot-detection", + ), + pytest.param("plain text, no markup at all", id="plain-text"), + ], +) +def test_a_real_page_is_not_mistaken_for_a_challenge(html): + """The check must not fire on an article that merely discusses the topic. + + A paper about CAPTCHAs contains the words; a challenge page is short and + carries them in its title or as its whole body. Getting this backwards would + discard real references. + """ + assert URLSource._is_bot_challenge(html) is False