fix: resolve relative URLs in the urllib3 download backend - #909
Open
ebarkhordar wants to merge 1 commit into
Open
fix: resolve relative URLs in the urllib3 download backend#909ebarkhordar wants to merge 1 commit into
ebarkhordar wants to merge 1 commit into
Conversation
geturl() returns the raw Location header after a redirect and the request URI otherwise, both of which can be relative. The bare path then reaches Response.url, so probe_alternative_homepage() derives an empty base URL and focused_crawler() returns nothing for any non-root start URL. The pycurl backend uses EFFECTIVE_URL and is unaffected. Fixes adbar#696
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #909 +/- ##
=======================================
Coverage 99.71% 99.71%
=======================================
Files 21 21
Lines 4167 4168 +1
=======================================
+ Hits 4155 4156 +1
Misses 12 12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
focused_crawler()returns nothing for any non-root start URL when trafilatura runs on the urllib3 backend. No redirect is required, which is what makes it reproducible without the site from the report.Root cause
_send_urllib_requestbuilds the standardisedResponsefromresponse.geturl():For a
PoolManagerrequest,geturl()returnsretries.history[-1].redirect_locationwhen a redirect happened, which is the rawLocationheader and may legally be relative, and otherwise the request URI that was passed down to the connection pool. Both are bare paths._send_pycurl_requestusescurl.getinfo(pycurl.EFFECTIVE_URL), which is always absolute, so the two backends disagree and only the urllib3 one is wrong. That is also why the report could not be reproduced here: with pycurl installed, every case passes.Downstream,
probe_alternative_homepagereads the bare path as a redirect target and assigns it tohomepage(spider.py:134-136).get_base_url()then returns'', so the caller guardif htmlstring and homepage and new_base_urlis false andfocused_crawleryields empty results with no error. The second consumer,process_responseatspider.py:232, files the bare path intoURL_STOREas visited, so the real URL is never marked.Fix
urljoin(url, response.geturl() or url). It resolves a relativeLocationagainst the requested URL and is a no-op on an absolute one, so it reproduces the pycurl backend'sEFFECTIVE_URLsemantics and leaves redirect detection intact. Correcting it at the source covers both consumers.I did not take the
response.url not in homepagechange suggested in the issue: it patches one of the two call sites, and the reporter noted himself that it breaks redirect handling.Verification
tests/downloads_tests.py. On master the two relative cases fail (/news/news,/section/) and the absolute case passes as the no-op control; with the fix all three pass.cli_tests.py::test_sysoutput, which fails identically on unmodified master in the same container because it asserts/root/forbidden/is unwritable and the container runs as root.ruff check .,ruff format --check .,mypy -p trafilaturaandpython tests/eval_gate.pyall pass. The eval gate is unchanged at the pinned floors, as expected for a change that touches no extraction path.http.server, across no redirect, relativeLocationand absoluteLocation; patched urllib3 matches pycurl in all three. That path is not in the suite, becausemock_networkintests/conftest.pyreplaces_send_urllib_requestitself and a spider-level test would mock away the code under test. I ran Python 3.13 and 3.14, not 3.10.Fixes #696