Skip urllib3 SSRF redirect tests when urllib3 is not installed#1682
Conversation
TestRedirectRevalidation mocks urllib3.PoolManager and exercises
read_range, which internally calls _urllib3_timeout() and tries to
import urllib3 lazily. urllib3 is an optional runtime dependency
(_HTTPSource falls back to stdlib urllib.request when it's missing),
but the test class did not gate on its presence, so CI without
urllib3 saw four ModuleNotFoundError failures.
Add an autouse pytest.importorskip("urllib3") fixture on the class
so the urllib3-specific tests skip cleanly when the package is
absent. The stdlib redirect-handler tests in the same class remain
untouched.
The autouse fixture on TestRedirectRevalidation also skipped the three test_stdlib_* tests in the same class, which exercise the stdlib redirect handler directly and must run regardless of whether urllib3 is installed. Move the gate into each of the four urllib3 test methods individually so the stdlib tests stay live.
There was a problem hiding this comment.
Pull request overview
This PR fixes CI failures in the GeoTIFF SSRF-hardening test suite by ensuring urllib3-specific redirect revalidation tests are skipped when urllib3 (an optional dependency) is not installed.
Changes:
- Add
pytest.importorskip("urllib3")guards to eachtest_urllib3_*redirect revalidation test. - Add an explanatory comment clarifying the separation between urllib3-transport tests vs stdlib-transport tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # The test_urllib3_* tests exercise the urllib3 transport path: they mock | ||
| # urllib3.PoolManager and call read_range(), which internally builds a | ||
| # urllib3.Timeout via _urllib3_timeout(). urllib3 is an optional runtime | ||
| # dependency (_HTTPSource falls back to stdlib urllib.request when it's | ||
| # missing -- see _reader.py:615-617), so each urllib3-using test starts | ||
| # with pytest.importorskip("urllib3"). The test_stdlib_* tests below | ||
| # exercise the stdlib redirect handler directly and run regardless. |
There was a problem hiding this comment.
Thanks. I tried that first — an @pytest.fixture(autouse=True) pytest.importorskip("urllib3") at the class level. The trade-off is that an autouse fixture also fires for the three test_stdlib_* methods in the same class (the stdlib redirect handler tests that don't go through _urllib3_timeout), and those should keep running on machines without urllib3 installed.
A separate class-with-autouse-fixture for just the urllib3 subset would work but it requires moving four methods out, which felt heavier than inlining the four calls. I left a class-level comment to nudge future urllib3-path tests toward the same gate.
Closes #1681.
Summary
TestRedirectRevalidationinxrspatial/geotiff/tests/test_ssrf_hardening_1664.pyexercises the urllib3 transport path: each test mocksurllib3.PoolManagerand callssrc.read_range(...), which internally builds aurllib3.Timeoutvia_urllib3_timeout()(a lazyimport urllib3).urllib3is an optional runtime dep (_HTTPSourcefalls back to stdliburllib.requestwhen it's missing), but the test class did not gate on its presence.In CI environments without
urllib3, the fourtest_urllib3_*tests fail withModuleNotFoundError. Main has been red on this since commit 919d827 (#1668); every PR branched from main hits the same wall (PRs #1675-#1680).This adds an autouse
pytest.importorskip("urllib3")fixture to the class so urllib3-specific tests skip cleanly when the package is absent. The threetest_stdlib_*tests in the same class are unaffected.Test plan