fix: LinkExtractor never filters compound extensions like .tar.gz - #357
Closed
MrStarkEG wants to merge 5 commits into
Closed
fix: LinkExtractor never filters compound extensions like .tar.gz#357MrStarkEG wants to merge 5 commits into
MrStarkEG wants to merge 5 commits into
Conversation
`_url_extension` returned only the suffix after the last dot, so
`dataset.tar.gz` yielded "gz" — which isn't in `IGNORED_EXTENSIONS`
(only "tar.gz" is). The deny check therefore never matched, and any
compound extension passed to `deny_extensions` was silently ignored.
Return the set of all dot-suffixes ({"gz", "tar.gz"}) and reject when
any intersects the deny list. Single-part extensions are unaffected.
Closes D4Vinci#349
There was a problem hiding this comment.
Pull request overview
This PR fixes LinkExtractor’s extension filtering so compound extensions (e.g., .tar.gz) are correctly recognized and denied, aligning behavior with IGNORED_EXTENSIONS and explicit deny_extensions values.
Changes:
- Update
_url_extension()to return all dot-suffixes of the last path segment (enabling compound extension matching). - Update the deny check to reject URLs when any returned suffix intersects
deny_extensions. - Add regression tests covering default compound denial, explicit compound denial, and case-insensitivity.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scrapling/spiders/links.py |
Fix compound extension detection and use set intersection for deny filtering. |
tests/spiders/test_links.py |
Add regression tests for compound extension filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert urls == ["https://example.com/a.pdf"] | ||
|
|
||
| def test_default_deny_extensions_drops_compound_tar_gz(self): | ||
| # tar.gz is in IGNORED_EXTENSIONS but the last-dot-only suffix is "gz" |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+156
to
+159
| return set() | ||
| parts = last.lower().split(".") | ||
| # Return all dot-suffixes so compound extensions like "tar.gz" are matched | ||
| return {".".join(parts[i:]) for i in range(1, len(parts))} |
Comment on lines
+199
to
+201
| def test_default_deny_extensions_drops_compound_tar_gz(self): | ||
| # tar.gz is in IGNORED_EXTENSIONS but the last-dot-only suffix is "gz" | ||
| html = '<a href="/d/dataset.tar.gz">tgz</a><a href="/d/data.tar.bz2">tbz</a><a href="/d">ok</a>' |
Filter empty path segments so "file." yields no extension and "a..zip" resolves to "zip" rather than ".zip". Refresh a stale test comment and add a regression test for the dot edge cases.
Protego 0.6.1 fixed `Request-rate: 1/10` parsing to return RequestRate(requests=1, seconds=10); earlier versions returned (1, 1). get_delay_directives now yields (1, 10), so this test broke on CI once protego 0.6.1 shipped. Assert the correct value.
Comment on lines
171
to
+175
| c_delay, r_rate = await mgr.get_delay_directives("https://example.com/", "s1") | ||
|
|
||
| assert c_delay is None | ||
| assert r_rate is not None | ||
| assert r_rate == (1, 1) | ||
| assert r_rate == (1, 10) |
Comment on lines
+199
to
+222
| def test_default_deny_extensions_drops_compound_tar_gz(self): | ||
| # Regression: compound extensions in IGNORED_EXTENSIONS (e.g. "tar.gz") must be dropped | ||
| html = '<a href="/d/dataset.tar.gz">tgz</a><a href="/d/data.tar.bz2">tbz</a><a href="/d">ok</a>' | ||
| resp = _make_response(html) | ||
| urls = LinkExtractor().extract(resp) | ||
| assert urls == ["https://example.com/d"] | ||
|
|
||
| def test_custom_compound_deny_extension(self): | ||
| html = '<a href="/dataset.tar.gz">tgz</a><a href="/plain.gz">gz</a>' | ||
| resp = _make_response(html) | ||
| # only the compound ext is denied; a plain .gz must still pass | ||
| urls = LinkExtractor(deny_extensions=["tar.gz"]).extract(resp) | ||
| assert urls == ["https://example.com/plain.gz"] | ||
|
|
||
| def test_compound_deny_is_case_insensitive(self): | ||
| ex = LinkExtractor() | ||
| assert ex.matches("https://example.com/archive.TAR.GZ") is False | ||
|
|
||
| def test_trailing_and_double_dots_yield_no_spurious_extension(self): | ||
| # "file." and "a..zip" must not produce empty or ".zip" suffixes | ||
| ex = LinkExtractor() | ||
| assert ex.matches("https://example.com/file.") is True | ||
| assert ex.matches("https://example.com/a..zip") is False | ||
|
|
Owner
|
Hi, thanks for your contribution, but this PR was made against |
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.
Fixes #349.
_url_extension()returned only the suffix after the last dot, sodataset.tar.gzyielded"gz"— which is not inIGNORED_EXTENSIONS(only"tar.gz"is). The deny check atlinks.pynever matched, so.tar.gzlinks were kept and passingdeny_extensions=["tar.gz"]explicitly was silently ignored too.Fix:
_url_extension()now returns the set of every dot-suffix of the last path segment ({"gz", "tar.gz"}), and the filter rejects the URL when any suffix intersectsdeny_extensions. Single-part extensions are unchanged.Added 4 regression tests: default compound drops, custom compound deny (plain
.gzstill passes), case-insensitivity, and trailing/double-dot edge cases (file.,a..zip).Note — unrelated CI fix (separate commit
afbb0a7):tests/spiders/test_robotstxt.pyexpectedRequest-rate: 1/10→(1, 1). Protego 0.6.1 corrected that parse to(1, 10), which broke the test on CI independently of this PR. Updated the expectation to(1, 10)(also latently broken onmain).