mirror of
https://github.com/ThePhaseless/Byparr.git
synced 2026-11-10 11:08:20 +00:00
fix: read the page after the idle wait, not before it
When no Cloudflare interstitial was detected, _navigate_and_solve() snapshotted page.content() right after domcontentloaded and build_response_content() returned that snapshot, even though _wait_for_networkidle() ran afterwards. A site with its own JS guard (com-x.life: 302 to a proof-of-work page that sets cookies and calls location.replace) finishes its redirect during that wait, so Byparr returned the guard page while solution.url already named the target. The snapshot came in with the localized-detection change (#388) only to reuse HTML that had already been read; before it the body was always read after the idle wait. Drop it, along with the challenge_detected and page_html plumbing, and read the settled page on both branches. The early read was also a liability of its own: a guard redirecting within a few hundred milliseconds of domcontentloaded makes that read fail with "Failed to find execution context", which escaped read_item as a 500. The late read can still race a navigation that lands right at the budget's edge, so if it fails, wait for network idle again and read once more. The error is caught as invisible_playwright's Error, which is what the browser actually raises. Fixes #418. Thanks to c4uran for the analysis. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
838704ea23
commit
c653b10533
3 files changed
+57
-25
No files matched your search
+16
-10
@@ -1,18 +1,18 @@
|
||||
import base64
|
||||
from contextlib import suppress
|
||||
|
||||
from invisible_playwright.async_api import Error
|
||||
from playwright.async_api import Page
|
||||
|
||||
from src.models import LinkRequest
|
||||
from src.utils import logger
|
||||
from src.utils import TimeoutTimer, logger, remaining_ms
|
||||
|
||||
|
||||
async def build_response_content(
|
||||
page: Page,
|
||||
request: LinkRequest,
|
||||
page_request: object,
|
||||
*,
|
||||
challenge_detected: bool,
|
||||
page_html: str | None,
|
||||
timer: TimeoutTimer,
|
||||
) -> tuple[str, str]:
|
||||
"""Build (content_type, response_content) from the settled page."""
|
||||
if request.return_only_cookies:
|
||||
@@ -23,12 +23,18 @@ async def build_response_content(
|
||||
):
|
||||
return await fetch_pdf_content(page)
|
||||
|
||||
response_content = (
|
||||
page_html
|
||||
if page_html is not None and not challenge_detected
|
||||
else await page.content()
|
||||
)
|
||||
return "text/html", response_content
|
||||
return "text/html", await read_settled_html(page, timer)
|
||||
|
||||
|
||||
async def read_settled_html(page: Page, timer: TimeoutTimer) -> str:
|
||||
"""Read the page's HTML, reading it again if a navigation pulled it away."""
|
||||
try:
|
||||
return await page.content()
|
||||
except Error:
|
||||
logger.info("Page navigated while being read; reading it again once idle")
|
||||
with suppress(Error):
|
||||
await page.wait_for_load_state("networkidle", timeout=remaining_ms(timer))
|
||||
return await page.content()
|
||||
|
||||
|
||||
async def fetch_pdf_content(page: Page) -> tuple[str, str]:
|
||||
|
||||
+7
-15
@@ -66,9 +66,7 @@ async def read_item(request: LinkRequest, dep: BrowserDep) -> LinkResponse:
|
||||
await setup_routes(request, dep)
|
||||
|
||||
try:
|
||||
challenge_detected, page_html, page_request = await _navigate_and_solve(
|
||||
dep, request, timer
|
||||
)
|
||||
page_request = await _navigate_and_solve(dep, request, timer)
|
||||
except (TimeoutError, PlaywrightTimeoutError) as e:
|
||||
logger.error("Timed out while loading the page or solving the challenge")
|
||||
raise HTTPException(
|
||||
@@ -90,8 +88,7 @@ async def read_item(request: LinkRequest, dep: BrowserDep) -> LinkResponse:
|
||||
dep.page,
|
||||
request,
|
||||
page_request,
|
||||
challenge_detected=challenge_detected,
|
||||
page_html=page_html,
|
||||
timer,
|
||||
)
|
||||
|
||||
user_agent = (
|
||||
@@ -130,22 +127,17 @@ async def _navigate_and_solve(
|
||||
dep: BrowserDep,
|
||||
request: LinkRequest,
|
||||
timer: TimeoutTimer,
|
||||
) -> tuple[bool, str | None, object]:
|
||||
"""Navigate to the URL, then solve a challenge or wait for network idle."""
|
||||
page_html: str | None = None
|
||||
) -> object:
|
||||
"""Navigate to the URL, solve a challenge if one is up, then wait for network idle."""
|
||||
page_request = await dep.page.goto(request.url, timeout=remaining_ms(timer))
|
||||
await dep.page.wait_for_load_state(
|
||||
state="domcontentloaded", timeout=remaining_ms(timer)
|
||||
)
|
||||
|
||||
if not await challenge_present(dep.page):
|
||||
page_html = await dep.page.content()
|
||||
await _wait_for_networkidle(dep, timer)
|
||||
return False, page_html, page_request
|
||||
|
||||
await solve_challenge(dep.page, timer)
|
||||
if await challenge_present(dep.page):
|
||||
await solve_challenge(dep.page, timer)
|
||||
await _wait_for_networkidle(dep, timer)
|
||||
return True, page_html, page_request
|
||||
return page_request
|
||||
|
||||
|
||||
async def _wait_for_networkidle(dep: BrowserDep, timer: TimeoutTimer) -> None:
|
||||
|
||||
@@ -6,6 +6,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||
import httpx2
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
from invisible_playwright.async_api import Error as InvisiblePlaywrightError
|
||||
from playwright.async_api import Error as PlaywrightError
|
||||
from playwright.async_api import TimeoutError as PlaywrightTimeoutError
|
||||
from starlette.testclient import TestClient
|
||||
@@ -203,6 +204,39 @@ async def test_networkidle_timeout_after_domcontentloaded_returns_content():
|
||||
assert response.solution.response == "<html><title>Login</title></html>"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_html_is_read_after_a_js_redirect_lands():
|
||||
"""A non-Cloudflare guard that redirects during the idle wait must not leak (#418)."""
|
||||
dep = fake_dep()
|
||||
dep.page.content.return_value = "<html><title>Checking</title></html>"
|
||||
load_state = dep.page.wait_for_load_state.side_effect
|
||||
|
||||
def redirect_while_idling(state: str, **kwargs: object) -> None:
|
||||
load_state(state, **kwargs)
|
||||
if state == "networkidle":
|
||||
dep.page.content.return_value = "<html><title>Article</title></html>"
|
||||
|
||||
dep.page.wait_for_load_state.side_effect = redirect_while_idling
|
||||
|
||||
response = await read_item(LinkRequest(url="https://example.test/login"), dep)
|
||||
|
||||
assert response.solution.response == "<html><title>Article</title></html>"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_html_read_racing_a_navigation_is_retried():
|
||||
"""A read that loses its document to a navigation reads the new one instead."""
|
||||
dep = fake_dep()
|
||||
dep.page.content.side_effect = [
|
||||
InvisiblePlaywrightError("Failed to find execution context with id = id-5"),
|
||||
"<html><title>Article</title></html>",
|
||||
]
|
||||
|
||||
response = await read_item(LinkRequest(url="https://example.test/login"), dep)
|
||||
|
||||
assert response.solution.response == "<html><title>Article</title></html>"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_domcontentloaded_timeout_returns_408():
|
||||
"""Fatal timeouts during initial page load still return a controlled 408."""
|
||||
|
||||
Reference in new issue
Block a user