mirror of https://github.com/scrapy/scrapy.git
Fix Proxy-Authorization handling in BaseStreamingDownloadHandler (#7630)
This commit is contained in:
parent
b7824db573
commit
fada8be1db
37
conftest.py
37
conftest.py
|
|
@ -74,40 +74,19 @@ def mockserver() -> Generator[MockServer]:
|
|||
|
||||
|
||||
@pytest.fixture # function scope because it modifies os.environ
|
||||
def mitm_proxy_server(monkeypatch: pytest.MonkeyPatch) -> Generator[MitmProxy]:
|
||||
proxy = MitmProxy()
|
||||
def proxy_server(
|
||||
request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch
|
||||
) -> Generator[str]:
|
||||
kind = request.param
|
||||
proxy = MitmProxy(mode="socks5" if kind == "socks5" else None)
|
||||
url = proxy.start()
|
||||
if kind == "https":
|
||||
url = url.replace("http://", "https://")
|
||||
monkeypatch.setenv("http_proxy", url)
|
||||
monkeypatch.setenv("https_proxy", url)
|
||||
|
||||
try:
|
||||
yield proxy
|
||||
finally:
|
||||
proxy.stop()
|
||||
|
||||
|
||||
@pytest.fixture # function scope because it modifies os.environ
|
||||
def mitm_proxy_server_https(monkeypatch: pytest.MonkeyPatch) -> Generator[MitmProxy]:
|
||||
proxy = MitmProxy()
|
||||
url = proxy.start().replace("http://", "https://")
|
||||
monkeypatch.setenv("http_proxy", url)
|
||||
monkeypatch.setenv("https_proxy", url)
|
||||
|
||||
try:
|
||||
yield proxy
|
||||
finally:
|
||||
proxy.stop()
|
||||
|
||||
|
||||
@pytest.fixture # function scope because it modifies os.environ
|
||||
def socks5_proxy_server(monkeypatch: pytest.MonkeyPatch) -> Generator[MitmProxy]:
|
||||
proxy = MitmProxy(mode="socks5")
|
||||
url = proxy.start()
|
||||
monkeypatch.setenv("http_proxy", url)
|
||||
monkeypatch.setenv("https_proxy", url)
|
||||
|
||||
try:
|
||||
yield proxy
|
||||
yield kind
|
||||
finally:
|
||||
proxy.stop()
|
||||
|
||||
|
|
|
|||
|
|
@ -241,6 +241,16 @@ class BaseStreamingDownloadHandler(BaseHttpDownloadHandler, ABC, Generic[_Respon
|
|||
body=response_body.getvalue(),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _request_headers(request: Request) -> Headers:
|
||||
"""Get a prepared copy of the request headers.
|
||||
|
||||
This removes the Proxy-Authorization header.
|
||||
"""
|
||||
headers = request.headers.copy()
|
||||
headers.pop(b"Proxy-Authorization", None)
|
||||
return headers
|
||||
|
||||
def _get_bind_address_host(self) -> str | None:
|
||||
"""Return the host portion of the bind address.
|
||||
|
||||
|
|
@ -279,10 +289,8 @@ class BaseStreamingDownloadHandler(BaseHttpDownloadHandler, ABC, Generic[_Respon
|
|||
if not proxy:
|
||||
return None, None
|
||||
proxy = add_http_if_no_scheme(proxy)
|
||||
auth_header: list[bytes] | None = request.headers.pop(
|
||||
b"Proxy-Authorization", None
|
||||
)
|
||||
return proxy, auth_header[0].decode("ascii") if auth_header else None
|
||||
auth_header: bytes | None = request.headers.get(b"Proxy-Authorization")
|
||||
return proxy, auth_header.decode("ascii") if auth_header else None
|
||||
|
||||
def _extract_proxy_url_with_creds(self, request: Request) -> str | None:
|
||||
"""Return the proxy URL with the userinfo added based on the
|
||||
|
|
|
|||
|
|
@ -152,13 +152,14 @@ class HttpxDownloadHandler(_Base):
|
|||
f"SOCKS proxy support in {type(self).__name__} requires the 'httpx[socks]' extra to be installed."
|
||||
)
|
||||
client = self._get_client(proxy)
|
||||
headers = self._request_headers(request).to_tuple_list()
|
||||
|
||||
try:
|
||||
async with client.stream(
|
||||
request.method,
|
||||
request.url,
|
||||
content=request.body,
|
||||
headers=request.headers.to_tuple_list(),
|
||||
headers=headers,
|
||||
timeout=timeout,
|
||||
) as response:
|
||||
yield response
|
||||
|
|
|
|||
|
|
@ -1328,6 +1328,9 @@ class TestHttpProxyBase(ABC):
|
|||
assert response.body == self.expected_http_proxy_request_body
|
||||
|
||||
|
||||
PROXY_KINDS = ["http", "https", "socks5"]
|
||||
|
||||
|
||||
class TestMitmProxyBase(ABC):
|
||||
# whether the handler supports HTTPS proxies with HTTPS destinations
|
||||
handler_supports_tls_in_tls: bool = True
|
||||
|
|
@ -1338,57 +1341,54 @@ class TestMitmProxyBase(ABC):
|
|||
def settings_dict(self) -> dict[str, Any] | None:
|
||||
raise NotImplementedError
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@pytest.mark.usefixtures("mitm_proxy_server")
|
||||
@coroutine_test
|
||||
async def test_http_proxy(
|
||||
self, caplog: pytest.LogCaptureFixture, mockserver: MockServer, https_dest: bool
|
||||
) -> None:
|
||||
"""HTTP proxy, HTTP or HTTPS destination."""
|
||||
crawler = get_crawler(SingleRequestSpider, self.settings_dict)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
await crawler.crawl_async(
|
||||
seed=mockserver.url("/status?n=200", is_secure=https_dest)
|
||||
)
|
||||
assert isinstance(crawler.spider, SingleRequestSpider)
|
||||
self._assert_got_response_code(200, caplog.text)
|
||||
self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@pytest.mark.usefixtures("mitm_proxy_server_https")
|
||||
@coroutine_test
|
||||
async def test_https_proxy(
|
||||
self, caplog: pytest.LogCaptureFixture, mockserver: MockServer, https_dest: bool
|
||||
) -> None:
|
||||
"""HTTPS proxy, HTTP or HTTPS destination."""
|
||||
if https_dest and not self.handler_supports_tls_in_tls:
|
||||
def _maybe_skip(self, proxy_kind: str, https_dest: bool) -> None:
|
||||
if proxy_kind == "socks5" and not self.handler_supports_socks:
|
||||
pytest.skip("SOCKS proxies are not supported")
|
||||
if (
|
||||
proxy_kind == "https"
|
||||
and https_dest
|
||||
and not self.handler_supports_tls_in_tls
|
||||
):
|
||||
pytest.skip("HTTPS proxies for HTTPS destinations are not supported")
|
||||
crawler = get_crawler(SingleRequestSpider, self.settings_dict)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
await crawler.crawl_async(
|
||||
seed=mockserver.url("/status?n=200", is_secure=https_dest)
|
||||
)
|
||||
assert isinstance(crawler.spider, SingleRequestSpider)
|
||||
self._assert_got_response_code(200, caplog.text)
|
||||
self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest)
|
||||
|
||||
@pytest.mark.parametrize("proxy_server", PROXY_KINDS, indirect=True)
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@pytest.mark.usefixtures("mitm_proxy_server")
|
||||
@coroutine_test
|
||||
async def test_http_proxy_auth_error(
|
||||
async def test_proxy(
|
||||
self,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
proxy_server: str,
|
||||
mockserver: MockServer,
|
||||
https_dest: bool,
|
||||
) -> None:
|
||||
"""HTTP proxy, HTTP or HTTPS destination, wrong proxy creds."""
|
||||
"""HTTP/HTTPS/SOCKS5 proxy, HTTP or HTTPS destination."""
|
||||
self._maybe_skip(proxy_server, https_dest)
|
||||
crawler = get_crawler(SingleRequestSpider, self.settings_dict)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
await crawler.crawl_async(
|
||||
seed=mockserver.url("/status?n=200", is_secure=https_dest)
|
||||
)
|
||||
assert isinstance(crawler.spider, SingleRequestSpider)
|
||||
self._assert_got_response_code(200, caplog.text)
|
||||
self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest)
|
||||
|
||||
@pytest.mark.parametrize("proxy_server", PROXY_KINDS, indirect=True)
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@coroutine_test
|
||||
async def test_proxy_auth_error(
|
||||
self,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
proxy_server: str,
|
||||
mockserver: MockServer,
|
||||
https_dest: bool,
|
||||
) -> None:
|
||||
"""HTTP/HTTPS/SOCKS5 proxy, HTTP or HTTPS destination, wrong proxy creds."""
|
||||
self._maybe_skip(proxy_server, https_dest)
|
||||
envvar = "https_proxy" if https_dest else "http_proxy"
|
||||
monkeypatch.setenv(envvar, wrong_credentials(os.environ[envvar]))
|
||||
crawler = get_crawler(SimpleSpider, self.settings_dict)
|
||||
|
|
@ -1396,20 +1396,28 @@ class TestMitmProxyBase(ABC):
|
|||
await crawler.crawl_async(
|
||||
mockserver.url("/status?n=200", is_secure=https_dest)
|
||||
)
|
||||
# The proxy returns a 407 error code but it does not reach the client;
|
||||
# it just sees an exception.
|
||||
self._assert_got_auth_exception(caplog.text)
|
||||
if proxy_server == "socks5":
|
||||
assert "DownloadConnectionRefusedError" in caplog.text
|
||||
else:
|
||||
# The proxy returns a 407 error code but it does not reach the
|
||||
# client; it just sees an exception.
|
||||
self._assert_got_auth_exception(caplog.text)
|
||||
|
||||
@pytest.mark.parametrize("proxy_server", PROXY_KINDS, indirect=True)
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@pytest.mark.usefixtures("mitm_proxy_server")
|
||||
@coroutine_test
|
||||
async def test_dont_leak_proxy_authorization_header(
|
||||
self, caplog: pytest.LogCaptureFixture, mockserver: MockServer, https_dest: bool
|
||||
async def test_proxy_dont_leak_auth_header(
|
||||
self,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
proxy_server: str,
|
||||
mockserver: MockServer,
|
||||
https_dest: bool,
|
||||
) -> None:
|
||||
"""HTTP proxy, HTTP or HTTPS destination. Check that the auth header
|
||||
is not sent to the destination."""
|
||||
"""HTTP/HTTPS/SOCKS5 proxy, HTTP or HTTPS destination. Check that the
|
||||
auth header is not sent to the destination."""
|
||||
self._maybe_skip(proxy_server, https_dest)
|
||||
request = Request(mockserver.url("/echo", is_secure=https_dest))
|
||||
crawler = get_crawler(SingleRequestSpider, self.settings_dict)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
|
|
@ -1420,48 +1428,36 @@ class TestMitmProxyBase(ABC):
|
|||
echo = json.loads(crawler.spider.meta["responses"][0].text)
|
||||
assert "Proxy-Authorization" not in echo["headers"]
|
||||
|
||||
@pytest.mark.parametrize("proxy_server", PROXY_KINDS, indirect=True)
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@pytest.mark.usefixtures("socks5_proxy_server")
|
||||
@coroutine_test
|
||||
async def test_download_with_socks_proxy(
|
||||
self, caplog: pytest.LogCaptureFixture, mockserver: MockServer, https_dest: bool
|
||||
) -> None:
|
||||
"""SOCKS5 proxy, HTTP or HTTPS destination."""
|
||||
if not self.handler_supports_socks:
|
||||
pytest.skip("SOCKS proxies are not supported")
|
||||
crawler = get_crawler(SingleRequestSpider, self.settings_dict)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
await crawler.crawl_async(
|
||||
seed=mockserver.url("/status?n=200", is_secure=https_dest)
|
||||
)
|
||||
assert isinstance(crawler.spider, SingleRequestSpider)
|
||||
self._assert_got_response_code(200, caplog.text)
|
||||
self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"]
|
||||
)
|
||||
@pytest.mark.usefixtures("socks5_proxy_server")
|
||||
@coroutine_test
|
||||
async def test_socks_proxy_auth_error(
|
||||
async def test_proxy_redirect(
|
||||
self,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
proxy_server: str,
|
||||
mockserver: MockServer,
|
||||
https_dest: bool,
|
||||
) -> None:
|
||||
if not self.handler_supports_socks:
|
||||
pytest.skip("SOCKS proxies are not supported")
|
||||
envvar = "https_proxy" if https_dest else "http_proxy"
|
||||
monkeypatch.setenv(envvar, wrong_credentials(os.environ[envvar]))
|
||||
crawler = get_crawler(SimpleSpider, self.settings_dict)
|
||||
"""HTTP/HTTPS/SOCKS5 proxy, HTTP or HTTPS destination, following a
|
||||
redirect. Check that the redirected request still goes through the
|
||||
proxy and doesn't lose the proxy auth.
|
||||
"""
|
||||
self._maybe_skip(proxy_server, https_dest)
|
||||
crawler = get_crawler(SingleRequestSpider, self.settings_dict)
|
||||
with caplog.at_level(logging.DEBUG):
|
||||
await crawler.crawl_async(
|
||||
mockserver.url("/status?n=200", is_secure=https_dest)
|
||||
seed=mockserver.url("/redirect", is_secure=https_dest)
|
||||
)
|
||||
assert "DownloadConnectionRefusedError" in caplog.text
|
||||
assert isinstance(crawler.spider, SingleRequestSpider)
|
||||
assert crawler.spider.meta.get("failure") is None
|
||||
responses = crawler.spider.meta.get("responses", [])
|
||||
assert len(responses) == 1
|
||||
assert responses[0].status == 200
|
||||
assert responses[0].url == mockserver.url("/redirected", is_secure=https_dest)
|
||||
self._assert_got_response_code(200, caplog.text)
|
||||
self._assert_headers(responses[0].headers, https_dest)
|
||||
|
||||
@staticmethod
|
||||
def _assert_headers(headers: Headers, https_dest: bool) -> None:
|
||||
|
|
|
|||
Loading…
Reference in New Issue