Verify hosts are alive before emitting wayback URLs - #3359
Conversation
Probe each scheme/host/port once with HEAD / before emitting URL_UNVERIFIEDs, caching verdicts in a bounded (~10MB) LRU. Dead hosts still populate the archive cache, since fetching their snapshots is what that feature is for.
📊 Performance Benchmark Report
📈 Detailed Results (All Benchmarks)
🎯 Performance Summary+ 1 improvement 🚀
! 2 regressions ⚠️
28 unchanged ✅🔍 Significant Changes (>10%)
🐍 Python Version 3.11.16 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #3359 +/- ##
======================================
+ Coverage 90% 90% +1%
======================================
Files 450 454 +4
Lines 46327 47177 +850
======================================
+ Hits 41588 42406 +818
- Misses 4739 4771 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Good change. Gating on (scheme, netloc) is the right granularity. Full wayback suite passes on the branch: 20 in 191s.
PR body is solid: the _archive_cache ordering, the _parameter_cache reachability argument, and punting the passive/active flag to #3358 all saved me work.
Two blockers inline.
| async def _live_netlocs(self, keys): | ||
| """Return the subset of (scheme, netloc) keys that answer HTTP. | ||
|
|
||
| Any response counts as alive -- only connect failures and timeouts (after retries) |
There was a problem hiding this comment.
🔴 Blocking. response is not None conflates "host answered" with "something in the path answered." With a proxy configured (web.py:128 applies it), a 502 for an unreachable upstream reads live while a direct connect failure reads dead. Same target, opposite verdict from proxy config alone.
Also: the 40K LRUCache is per module instance per scan, so 9.44 MB is per-scan.
Also pins the interesting-file exemption with a test: those findings are fetched from archive.org rather than the host, so they're intentionally not liveness-gated.
|
Both fixed. Proxy: liveness probing is now skipped entirely when Interesting files: no behavior change, dead hosts emitting those findings is the point of the feature. A backup.zip that's gone from the host but still on archive.org is exactly what it's for. Pinned with Also corrected the cache figure in the body to say per scan. 22 passed. |
singlerider
left a comment
There was a problem hiding this comment.
Re-reviewed at 1ffd801, both blockers resolved. Full wayback suite passes on the branch: 22 in 128s.
🟢 Proxy fix
Skipping the probe entirely is the right call over trying to interpret a proxied response. TestWaybackProxySkipsLivenessProbe asserts no probe is sent, the cache stays empty, and the URL is emitted anyway, which is the behavior that matters. RESOLVED ✅
🟢 Interesting-file exemption
Your reasoning is right and I withdraw the objection. TestWaybackDeadHostInterestingFile pins it exactly where it needed pinning: dead host, no URL_UNVERIFIED, finding still emitted. The PR body now states it next to the archive exemption. RESOLVED ✅
🔴 Blocking, _liveness_probing reads the raw config instead of the resolved proxy
setup() gates on self.scan.config.get("web", {}).get("http_proxy"), but the value that decides whether a request is actually proxied is WebHelper._http_proxy combined with _http_proxy_exclude (web.py:127-128). A user with http_proxy set and the target in http_proxy_exclude gets no probing at all, even though those probes would go direct and return a trustworthy verdict. Read the helper's resolved state rather than the raw config key.
🔴 Blocking, _live_netlocs runs per query call, not per scan
The LRUCache dedupes across calls, but the verbose line and the request_batch_stream dispatch are inside query(), so a scan with many in-scope domains issues a separate batch per domain at 25 concurrency each. On a large multi-domain scan that is a lot of concurrent probe batches against overlapping infrastructure. Worth either hoisting the batch or bounding it with the module's existing concurrency controls.
🟢 Test coverage
TestWaybackDeadHostSkip asserting the exact probe list, not just the emitted URLs, is what makes the once-per-host claim verifiable. Good test.
Problem
With
urls=True, wayback emits aURL_UNVERIFIEDfor every URL that survives filtering, with no check that the host still exists. A CDX response for a large domain routinely contains hundreds of hosts that are long dead, and every one of their URLs gets emitted for the rest of the scan to chase:httprequests each one, excavate parses the results, the spider follows them. That cost is large and entirely wasted.Nothing in the emission path checked liveness.
_pre_process_urlsapplies an in-scope test, but only for the archive/parameter/interesting-file metadata; theURL_UNVERIFIEDlist bypassed it.abort_ifruns after DNS resolution but only requires thein-scopetag, which an unresolvable host still carries.Change
Probe each
(scheme, netloc)once withHEAD /before emitting any of its URLs, and drop the URLs of hosts that don't answer.request_batch_streamat 25 concurrency,follow_redirects=False, so it's one request per host regardless of URL count.retries=1rather than inheritingweb.http_retries, so a single dropped packet can't condemn a live host even if a user sets that to 0.LRUCache(maxsize=40000), measured at 9.44 MB when full with realistic hostnames (248 bytes/entry). The cache lives on the module instance, so that figure is per scan.Only reachable from the
urls=Truebranch, so the default config sends no probes.The archive path is deliberately unaffected
_archive_cacheis populated before the liveness gate. Dead hosts are exactly what the archive feature exists for, sofinish()still fetches every snapshot it would have before. Eviction behavior is unchanged too: it only fires on a live 2xx URL event, which a dead host never produced anyway. The six existing archive tests all run againsthttp://127.0.0.1:1/..., i.e. dead hosts, and pass unchanged._parameter_cacheis now only populated for live hosts. Those entries could only ever be read when a live 2xx URL event arrived, so nothing is lost.Interesting-file findings are deliberately unaffected
Same reasoning as the archive path, and worth stating explicitly since the gate sits between them.
interesting_filesis built in_pre_process_urls, and_check_interesting_filesfetches fromweb.archive.org, not from the host. Abackup.zipthat no longer exists on a dead host is exactly what makes the archive worth searching, so those FINDINGs are emitted regardless of liveness.Pinned by
TestWaybackDeadHostInterestingFile, which asserts a dead host emits noURL_UNVERIFIEDevents but still produces the finding.Tests
New
TestWaybackDeadHostSkip: two hosts with identical DNS, only one answering HTTP. Asserts only the live host's URLs are emitted, that the host is probed exactly once regardless of URL count, and the resulting cache contents.Seven existing tests needed a liveness mock, since their hosts are mock-intercepted and an unmatched URL now reads as dead.
TestWaybackParametersgot an explicit/handler rather than relying on the httpserver's 500-fallback.22 passed, 0 failed.
Notes
httpwould have failed the same connection.urls=Truewhile it's still flaggedpassive. That's a pre-existing inconsistency (_is_http_wildcard_hostalready probes targets in every config) and is tracked separately in Module flags are static, but some modules are active or passive depending on config #3358 rather than fixed here.