Change HTTPConnection.getresponse() to return urllib3.HTTPResponse
ADD
changelog/2648.feature.rst
+Modified ``urllib3.HTTPConnection.getresponse`` to return an instance of ``urllib3.HTTPResponse`` instead of ``http.client.HTTPResponse``.
+
+Removed ``urllib3.HTTPResponse.from_httplib``.

RENAME
ci/requests.patch
ci/0003-requests-removed-warnings.patch
ADD
ci/0004-requests-chunked-requests.patch
+diff --git a/tests/test_lowlevel.py b/tests/test_lowlevel.py
+index 859d07e8..91fbce7c 100644
+--- a/tests/test_lowlevel.py
++++ b/tests/test_lowlevel.py
+@@ -20,7 +20,7 @@ def echo_response_handler(sock):
+ ) % (len(request_content), request_content)
+ sock.send(text_200)
+
+-
++@pytest.mark.xfail
+ def test_chunked_upload():
+ """can safely send generators"""
+ close_server = threading.Event()
+@@ -35,7 +35,7 @@ def test_chunked_upload():
+ assert r.status_code == 200
+ assert r.request.headers["Transfer-Encoding"] == "chunked"
+
+-
++@pytest.mark.xfail
+ def test_chunked_encoding_error():
+ """get a ChunkedEncodingError if the server returns a bad response"""
+
+@@ -59,7 +59,7 @@ def test_chunked_encoding_error():
+ requests.get(url)
+ close_server.set() # release server block
+
+-
++@pytest.mark.xfail
+ def test_chunked_upload_uses_only_specified_host_header():
+ """Ensure we use only the specified Host header for chunked requests."""
+ close_server = threading.Event()
+@@ -77,7 +77,7 @@ def test_chunked_upload_uses_only_specified_host_header():
+ assert expected_header in r.content
+ assert r.content.count(b"Host: ") == 1
+
+-
++@pytest.mark.xfail
+ def test_chunked_upload_doesnt_skip_host_header():
+ """Ensure we don't omit all Host headers with chunked requests."""
+ close_server = threading.Event()

MODIFY
noxfile.py
noxfile.py
@@ -116,7 +116,12 @@ def downstream_requests(session: nox.Session) -> None:
session.cd(tmp_dir)
git_clone(session, "https://github.com/psf/requests")
session.chdir("requests")
- session.run("git", "apply", f"{root}/ci/requests.patch", external=True)
+ session.run(
+ "git", "apply", f"{root}/ci/0003-requests-removed-warnings.patch", external=True
+ )
+ session.run(
+ "git", "apply", f"{root}/ci/0004-requests-chunked-requests.patch", external=True
+ )
session.run("git", "rev-parse", "HEAD", external=True)
session.install(".[socks]", silent=False)
session.install("-r", "requirements-dev.txt", silent=False)

MODIFY
src/urllib3/_request_methods.py
src/urllib3/_request_methods.py
@@ -85,8 +85,6 @@ class RequestMethods:
"""
method = method.upper()
- urlopen_kw["request_url"] = url
-
if json is not None and body is not None:
raise TypeError(
"request got values for both 'body' and 'json' parameters which are mutually exclusive"

MODIFY
src/urllib3/connection.py
src/urllib3/connection.py
@@ -6,6 +6,7 @@ import socket
import warnings
from http.client import HTTPConnection as _HTTPConnection
from http.client import HTTPException as HTTPException # noqa: F401
+from http.client import ResponseNotReady
from socket import timeout as SocketTimeout
from typing import (
IO,
@@ -25,9 +26,12 @@ from typing import (
if TYPE_CHECKING:
from typing_extensions import Literal
+ from .response import HTTPResponse
from .util.ssl_ import _TYPE_PEER_CERT_RET_DICT
from .util.ssltransport import SSLTransport
+from ._collections import HTTPHeaderDict
+from .util.response import assert_header_parsing
from .util.timeout import _DEFAULT_TIMEOUT, _TYPE_TIMEOUT, Timeout
from .util.util import to_str
@@ -45,6 +49,7 @@ except (ImportError, AttributeError):
from ._version import __version__
from .exceptions import (
ConnectTimeoutError,
+ HeaderParsingError,
NameResolutionError,
NewConnectionError,
ProxyError,
@@ -78,7 +83,6 @@ RECENT_DATE = datetime.date(2022, 1, 1)
_CONTAINS_CONTROL_CHAR_RE = re.compile(r"[^-!#$%&'*+.^_`|~0-9a-zA-Z]")
-
_TYPE_BODY = Union[bytes, IO[Any], Iterable[bytes], str]
@@ -89,6 +93,16 @@ class ProxyConfig(NamedTuple):
assert_fingerprint: Optional[str]
+class _ResponseOptions(NamedTuple):
+ # TODO: Remove this in favor of a better
+ # HTTP request/response lifecycle tracking.
+ request_method: str
+ request_url: str
+ preload_content: bool
+ decode_content: bool
+ enforce_content_length: bool
+
+
class HTTPConnection(_HTTPConnection):
"""
Based on :class:`http.client.HTTPConnection` but provides an extra constructor
@@ -135,6 +149,7 @@ class HTTPConnection(_HTTPConnection):
_tunnel_host: Optional[str]
_tunnel: ClassVar[Callable[["HTTPConnection"], None]]
_connecting_to_proxy: bool
+ _response_options: Optional[_ResponseOptions]
def __init__(
self,
@@ -167,6 +182,7 @@ class HTTPConnection(_HTTPConnection):
)
self._connecting_to_proxy = False
+ self._response_options = None
# https://github.com/python/mypy/issues/4125
# Mypy treats this as LSP violation, which is considered a bug.
@@ -293,8 +309,27 @@ class HTTPConnection(_HTTPConnection):
body: Optional[_TYPE_BODY] = None,
headers: Optional[Mapping[str, str]] = None,
chunked: bool = False,
+ preload_content: bool = True,
+ decode_content: bool = True,
+ enforce_content_length: bool = True,
) -> None:
+ # Store these values to be fed into the HTTPResponse
+ # object later. TODO: Remove this in favor of a real
+ # HTTP lifecycle mechanism.
+
+ # We have to store these before we call .request()
+ # because sometimes we can still salvage a response
+ # off the wire even if we aren't able to completely
+ # send the request body.
+ self._response_options = _ResponseOptions(
+ request_method=method,
+ request_url=url,
+ preload_content=preload_content,
+ decode_content=decode_content,
+ enforce_content_length=enforce_content_length,
+ )
+
if headers is None:
headers = {}
header_keys = frozenset(to_str(k.lower()) for k in headers)
@@ -372,6 +407,57 @@ class HTTPConnection(_HTTPConnection):
"""
self.request(method, url, body=body, headers=headers, chunked=True)
+ def getresponse( # type: ignore[override]
+ self,
+ ) -> "HTTPResponse":
+ """
+ Get the response from the server.
+
+ If the HTTPConnection is in the correct state, returns an instance of HTTPResponse or of whatever object is returned by the response_class variable.
+
+ If a request has not been sent or if a previous response has not be handled, ResponseNotReady is raised. If the HTTP response indicates that the connection should be closed, then it will be closed before the response is returned. When the connection is closed, the underlying socket is closed.
+ """
+ # Raise the same error as http.client.HTTPConnection
+ if self._response_options is None:
+ raise ResponseNotReady()
+
+ # Reset this attribute for being used again.
+ resp_options = self._response_options
+ self._response_options = None
+
+ # This is needed here to avoid circular import errors
+ from .response import HTTPResponse
+
+ # Get the response from http.client.HTTPConnection
+ httplib_response = super().getresponse()
+
+ try:
+ assert_header_parsing(httplib_response.msg)
+ except (HeaderParsingError, TypeError) as hpe:
+ log.warning(
+ "Failed to parse headers (url=%s): %s",
+ _url_from_connection(self, resp_options.request_url),
+ hpe,
+ exc_info=True,
+ )
+
+ headers = HTTPHeaderDict(httplib_response.msg.items())
+
+ response = HTTPResponse(
+ body=httplib_response,
+ headers=headers,
+ status=httplib_response.status,
+ version=httplib_response.version,
+ reason=httplib_response.reason,
+ preload_content=resp_options.preload_content,
+ decode_content=resp_options.decode_content,
+ original_response=httplib_response,
+ enforce_content_length=resp_options.enforce_content_length,
+ request_method=resp_options.request_method,
+ request_url=resp_options.request_url,
+ )
+ return response
+
class HTTPSConnection(HTTPConnection):
"""
@@ -749,3 +835,13 @@ if not ssl:
VerifiedHTTPSConnection = HTTPSConnection
+
+
+def _url_from_connection(
+ conn: Union[HTTPConnection, HTTPSConnection], path: Optional[str] = None
+) -> str:
+ """Returns the URL from a given connection. This is mainly used for testing and logging."""
+
+ scheme = "https" if isinstance(conn, HTTPSConnection) else "http"
+
+ return Url(scheme=scheme, host=conn.host, port=conn.port, path=path).url

MODIFY
src/urllib3/connectionpool.py
src/urllib3/connectionpool.py
@@ -4,7 +4,6 @@ import queue
import sys
import warnings
import weakref
-from http.client import HTTPResponse as _HttplibHTTPResponse
from socket import timeout as SocketTimeout
from types import TracebackType
from typing import TYPE_CHECKING, Any, Mapping, Optional, Type, TypeVar, Union, overload
@@ -27,7 +26,6 @@ from .exceptions import (
ClosedPoolError,
EmptyPoolError,
FullPoolError,
- HeaderParsingError,
HostChangedError,
InsecureRequestWarning,
LocationValueError,
@@ -39,11 +37,10 @@ from .exceptions import (
SSLError,
TimeoutError,
)
-from .response import BaseHTTPResponse, HTTPResponse
+from .response import HTTPResponse
from .util.connection import is_connection_dropped
from .util.proxy import connection_requires_http_tunnel
from .util.request import _TYPE_BODY_POSITION, set_file_position
-from .util.response import assert_header_parsing
from .util.retry import Retry
from .util.ssl_match_hostname import CertificateError
from .util.timeout import _DEFAULT_TIMEOUT, _TYPE_DEFAULT, Timeout
@@ -384,10 +381,16 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
conn: HTTPConnection,
method: str,
url: str,
+ body: Optional[_TYPE_BODY] = None,
+ headers: Optional[Mapping[str, str]] = None,
+ retries: Optional[Retry] = None,
timeout: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
chunked: bool = False,
- **httplib_request_kw: Any,
- ) -> _HttplibHTTPResponse:
+ response_conn: Optional[HTTPConnection] = None,
+ preload_content: bool = True,
+ decode_content: bool = True,
+ enforce_content_length: bool = True,
+ ) -> HTTPResponse:
"""
Perform a request on a given urllib connection object taken from our
pool.
@@ -395,12 +398,61 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
:param conn:
a connection from one of our connection pools
+ :param method:
+ HTTP request method (such as GET, POST, PUT, etc.)
+
+ :param url:
+ The URL to perform the request on.
+
+ :param body:
+ Data to send in the request body, either :class:`str`, :class:`bytes`,
+ an iterable of :class:`str`/:class:`bytes`, or a file-like object.
+
+ :param headers:
+ Dictionary of custom headers to send, such as User-Agent,
+ If-None-Match, etc. If None, pool headers are used. If provided,
+ these headers completely replace any pool-specific headers.
+
+ :param retries:
+ Configure the number of retries to allow before raising a
+ :class:`~urllib3.exceptions.MaxRetryError` exception.
+
+ Pass ``None`` to retry until you receive a response. Pass a
+ :class:`~urllib3.util.retry.Retry` object for fine-grained control
+ over different types of retries.
+ Pass an integer number to retry connection errors that many times,
+ but no other types of errors. Pass zero to never retry.
+
+ If ``False``, then retries are disabled and any exception is raised
+ immediately. Also, instead of raising a MaxRetryError on redirects,
+ the redirect response will be returned.
+
+ :type retries: :class:`~urllib3.util.retry.Retry`, False, or an int.
+
:param timeout:
- Socket timeout in seconds for the request. This can be a
- float or integer, which will set the same timeout value for
- the socket connect and the socket read, or an instance of
- :class:`urllib3.util.Timeout`, which gives you more fine-grained
- control over your timeouts.
+ If specified, overrides the default timeout for this one
+ request. It may be a float (in seconds) or an instance of
+ :class:`urllib3.util.Timeout`.
+
+ :param chunked:
+ If True, urllib3 will send the body using chunked transfer
+ encoding. Otherwise, urllib3 will send the body using the standard
+ content-length form. Defaults to False.
+
+ :param response_conn:
+ Set this to ``None`` if you will handle releasing the connection or
+ set the connection to have the response release it.
+
+ :param preload_content:
+ If True, the response's body will be preloaded during construction.
+
+ :param decode_content:
+ If True, will attempt to decode the body based on the
+ 'content-encoding' header.
+
+ :param enforce_content_length:
+ Enforce content length checking. Body returned by server must match
+ value of Content-Length header, if present. Otherwise, raise error.
"""
self.num_requests += 1
@@ -438,10 +490,16 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
# conn.request() calls http.client.*.request, not the method in
# urllib3.request. It also calls makefile (recv) on the socket.
try:
- if chunked:
- conn.request_chunked(method, url, **httplib_request_kw)
- else:
- conn.request(method, url, **httplib_request_kw)
+ conn.request(
+ method,
+ url,
+ body=body,
+ headers=headers,
+ chunked=chunked,
+ preload_content=preload_content,
+ decode_content=decode_content,
+ enforce_content_length=enforce_content_length,
+ )
# We are swallowing BrokenPipeError (errno.EPIPE) since the server is
# legitimately able to close the connection after sending a valid response.
@@ -472,11 +530,16 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
# Receive the response from the server
try:
- httplib_response = conn.getresponse()
+ response = conn.getresponse()
except (BaseSSLError, OSError) as e:
self._raise_timeout(err=e, url=url, timeout_value=read_timeout)
raise
+ # Set properties that are used by the pooling layer.
+ response.retries = retries
+ response._connection = response_conn
+ response._pool = self
+
log.debug(
'%s://%s:%s "%s %s %s" %s %s',
self.scheme,
@@ -486,24 +549,11 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
url,
# HTTP version
conn._http_vsn_str, # type: ignore[attr-defined]
- httplib_response.status,
- httplib_response.length,
- )
-
- try:
- assert_header_parsing(httplib_response.msg)
- except (HeaderParsingError, TypeError) as hpe:
- log.warning(
- "Failed to parse headers (url=%s): %s",
- self._absolute_url(url),
- hpe,
- exc_info=True,
+ response.status,
+ response.length_remaining,
)
- return httplib_response
-
- def _absolute_url(self, path: str) -> str:
- return Url(scheme=self.scheme, host=self.host, port=self.port, path=path).url
+ return response
def close(self) -> None:
"""
@@ -554,7 +604,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
chunked: bool = False,
body_pos: Optional[_TYPE_BODY_POSITION] = None,
**response_kw: Any,
- ) -> BaseHTTPResponse:
+ ) -> HTTPResponse:
"""
Get a connection from the pool and perform an HTTP request. This is the
lowest level call for making a request, so you'll need to specify all
@@ -645,7 +695,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
:param \\**response_kw:
Additional parameters are passed to
- :meth:`urllib3.response.HTTPResponse.from_httplib`
+ :meth:`urllib3.connection.HTTPConnection.getresponse`
"""
parsed_url = parse_url(url)
@@ -728,8 +778,14 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
)
raise
- # Make the request on the httplib connection object.
- httplib_response = self._make_request(
+ # If we're going to release the connection in ``finally:``, then
+ # the response doesn't need to know about the connection. Otherwise
+ # it will also try to release it and we'll have a double-release
+ # mess.
+ response_conn = conn if not release_conn else None
+
+ # Make the request on the HTTPConnection object
+ response = self._make_request(
conn,
method,
url,
@@ -737,23 +793,8 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
body=body,
headers=headers,
chunked=chunked,
- )
-
- # If we're going to release the connection in ``finally:``, then
- # the response doesn't need to know about the connection. Otherwise
- # it will also try to release it and we'll have a double-release
- # mess.
- response_conn = conn if not release_conn else None
-
- # Pass method to Response for length checking
- response_kw["request_method"] = method
-
- # Import httplib's response into our own wrapper object
- response = self.ResponseCls.from_httplib(
- httplib_response,
- pool=self,
- connection=response_conn,
retries=retries,
+ response_conn=response_conn,
**response_kw,
)
@@ -1126,6 +1167,13 @@ def _normalize_host(host: Optional[str], scheme: Optional[str]) -> Optional[str]
return host
+def _url_from_pool(
+ pool: Union[HTTPConnectionPool, HTTPSConnectionPool], path: Optional[str] = None
+) -> str:
+ """Returns the URL from a given connection pool. This is mainly used for testing and logging."""
+ return Url(scheme=pool.scheme, host=pool.host, port=pool.port, path=path).url
+
+
def _close_pool_connections(pool: "queue.LifoQueue[Any]") -> None:
"""Drains a queue of connections and closes each one."""
try:

MODIFY
src/urllib3/poolmanager.py
src/urllib3/poolmanager.py
@@ -468,7 +468,7 @@ class PoolManager(RequestMethods):
kw["headers"] = new_headers
try:
- retries = retries.increment(method, url, response=response, _pool=conn) # type: ignore[arg-type]
+ retries = retries.increment(method, url, response=response, _pool=conn)
except MaxRetryError:
if retries.raise_on_redirect:
response.drain_conn()

MODIFY
src/urllib3/response.py
src/urllib3/response.py
@@ -256,7 +256,7 @@ class BaseHTTPResponse(io.IOBase):
self.version = version
self.reason = reason
self.decode_content = decode_content
- self.request_url: Optional[str]
+ self._request_url: Optional[str] = request_url
self.retries = retries
self.chunked = False
@@ -301,6 +301,10 @@ class BaseHTTPResponse(io.IOBase):
def url(self) -> Optional[str]:
raise NotImplementedError()
+ @url.setter
+ def url(self, url: Optional[str]) -> None:
+ raise NotImplementedError()
+
@property
def closed(self) -> bool:
raise NotImplementedError()
@@ -309,6 +313,17 @@ class BaseHTTPResponse(io.IOBase):
def connection(self) -> Optional[HTTPConnection]:
raise NotImplementedError()
+ @property
+ def retries(self) -> Optional[Retry]:
+ return self._retries
+
+ @retries.setter
+ def retries(self, retries: Optional[Retry]) -> None:
+ # Override the request_url if retries has a redirect location.
+ if retries is not None and retries.history:
+ self.url = retries.history[-1].redirect_location
+ self._retries = retries
+
def stream(
self, amt: Optional[int] = 2**16, decode_content: Optional[bool] = None
) -> Iterator[bytes]:
@@ -487,10 +502,6 @@ class HTTPResponse(BaseHTTPResponse):
self._original_response = original_response
self._fp_bytes_read = 0
self.msg = msg
- if self.retries is not None and self.retries.history:
- self._request_url = self.retries.history[-1].redirect_location
- else:
- self._request_url = request_url
if body and isinstance(body, (str, bytes)):
self._body = body
@@ -820,33 +831,6 @@ class HTTPResponse(BaseHTTPResponse):
if data:
yield data
- @classmethod
- def from_httplib(
- ResponseCls: Type["HTTPResponse"], r: _HttplibHTTPResponse, **response_kw: Any
- ) -> "HTTPResponse":
- """
- Given an :class:`http.client.HTTPResponse` instance ``r``, return a
- corresponding :class:`urllib3.response.HTTPResponse` object.
-
- Remaining parameters are passed to the HTTPResponse constructor, along
- with ``original_response=r``.
- """
- headers = r.msg
-
- if not isinstance(headers, HTTPHeaderDict):
- headers = HTTPHeaderDict(headers.items()) # type: ignore[assignment]
-
- resp = ResponseCls(
- body=r,
- headers=headers, # type: ignore[arg-type]
- status=r.status,
- version=r.version,
- reason=r.reason,
- original_response=r,
- **response_kw,
- )
- return resp
-
# Overrides from io.IOBase
def close(self) -> None:
if not self.closed and self._fp:

MODIFY
test/test_connection.py
test/test_connection.py
import datetime
import socket
import typing
+from http.client import ResponseNotReady
from unittest import mock
import pytest
@@ -8,8 +9,10 @@ import pytest
from urllib3.connection import ( # type: ignore[attr-defined]
RECENT_DATE,
CertificateError,
+ HTTPConnection,
HTTPSConnection,
_match_hostname,
+ _url_from_connection,
_wrap_proxy_error,
)
from urllib3.exceptions import HTTPError, ProxyError
@@ -217,3 +220,16 @@ class TestConnection:
new_err = _wrap_proxy_error(HTTPError("unknown protocol"), proxy_scheme)
assert isinstance(new_err, ProxyError) is True
assert err_part in new_err.args[0]
+
+ def test_url_from_pool(self) -> None:
+ conn = HTTPConnection("google.com", port=80)
+
+ path = "path?query=foo"
+ assert f"http://google.com:80/{path}" == _url_from_connection(conn, path)
+
+ def test_getresponse_requires_reponseoptions(self) -> None:
+ conn = HTTPConnection("google.com", port=80)
+
+ # Should error if a request has not been sent
+ with pytest.raises(ResponseNotReady):
+ conn.getresponse()

MODIFY
test/test_connectionpool.py
test/test_connectionpool.py
@@ -11,10 +11,12 @@ from unittest.mock import Mock, patch
import pytest
from dummyserver.server import DEFAULT_CA
+from urllib3 import Retry
from urllib3.connection import HTTPConnection
from urllib3.connectionpool import (
HTTPConnectionPool,
HTTPSConnectionPool,
+ _url_from_pool,
connection_from_url,
)
from urllib3.exceptions import (
@@ -449,11 +451,10 @@ class TestConnectionPool:
assert old_pool_queue is not None
old_pool_queue.get(block=False)
- def test_absolute_url(self) -> None:
- with connection_from_url("http://google.com:80") as c:
- assert "http://google.com:80/path?query=foo" == c._absolute_url(
- "path?query=foo"
- )
+ def test_url_from_pool(self) -> None:
+ with connection_from_url("http://google.com:80") as pool:
+ path = "path?query=foo"
+ assert f"http://google.com:80/{path}" == _url_from_pool(pool, path)
def test_ca_certs_default_cert_required(self) -> None:
with connection_from_url("https://google.com:80", ca_certs=DEFAULT_CA) as pool:
@@ -506,17 +507,45 @@ class TestConnectionPool:
successful response on subsequent calls.
"""
- def __init__(self, ex: Type[BaseException]) -> None:
+ def __init__(
+ self, ex: Type[BaseException], pool: HTTPConnectionPool
+ ) -> None:
super().__init__()
self._ex: Optional[Type[BaseException]] = ex
-
- def __call__(self, *args: Any, **kwargs: Any) -> httplib.HTTPResponse:
+ self._pool = pool
+
+ def __call__(
+ self,
+ conn: HTTPConnection,
+ method: str,
+ url: str,
+ *args: Any,
+ retries: Retry,
+ **kwargs: Any,
+ ) -> HTTPResponse:
if self._ex:
ex, self._ex = self._ex, None
raise ex()
- response = httplib.HTTPResponse(MockSock) # type: ignore[arg-type]
- response.fp = MockChunkedEncodingResponse([b"f", b"o", b"o"]) # type: ignore[assignment]
- response.headers = response.msg = httplib.HTTPMessage()
+ httplib_response = httplib.HTTPResponse(MockSock) # type: ignore[arg-type]
+ httplib_response.fp = MockChunkedEncodingResponse([b"f", b"o", b"o"]) # type: ignore[assignment]
+ httplib_response.headers = httplib_response.msg = httplib.HTTPMessage()
+
+ response_conn: Optional[HTTPConnection] = kwargs.get("response_conn")
+
+ response = HTTPResponse(
+ body=httplib_response,
+ headers=httplib_response.headers, # type: ignore[arg-type]
+ status=httplib_response.status,
+ version=httplib_response.version,
+ reason=httplib_response.reason,
+ original_response=httplib_response,
+ retries=retries,
+ request_method=method,
+ request_url=url,
+ preload_content=False,
+ connection=response_conn,
+ pool=self._pool,
+ )
return response
def _test(exception: Type[BaseException]) -> None:
@@ -525,7 +554,9 @@ class TestConnectionPool:
# connection is left on the response object, instead of being
# released back into the pool.
with patch.object(
- pool, "_make_request", _raise_once_make_request_function(exception)
+ pool,
+ "_make_request",
+ _raise_once_make_request_function(exception, pool),
):
response = pool.urlopen(
"GET",
@@ -556,23 +587,4 @@ class TestConnectionPool:
with patch.object(Timeout, "read_timeout", 0):
timeout = Timeout(1, 1, 1)
with pytest.raises(ReadTimeoutError):
- pool._make_request(conn, "", "", timeout)
-
- def test_custom_http_response_class(self) -> None:
- class CustomHTTPResponse(HTTPResponse):
- pass
-
- class CustomConnectionPool(HTTPConnectionPool):
- ResponseCls = CustomHTTPResponse
-
- def _make_request(self, *args: Any, **kwargs: Any) -> httplib.HTTPResponse:
- httplib_response = httplib.HTTPResponse(MockSock) # type: ignore[arg-type]
- httplib_response.fp = MockChunkedEncodingResponse([b"f", b"o", b"o"]) # type: ignore[assignment]
- httplib_response.headers = httplib_response.msg = httplib.HTTPMessage()
- return httplib_response
-
- with CustomConnectionPool(host="localhost", maxsize=1, block=True) as pool:
- response = pool.request(
- "GET", "/", retries=False, chunked=True, preload_content=False
- )
- assert isinstance(response, CustomHTTPResponse)
+ pool._make_request(conn, "", "", timeout=timeout)

ADD
test/with_dummyserver/test_connection.py
+from http.client import ResponseNotReady
+from typing import Generator
+
+import pytest
+
+from dummyserver.testcase import HTTPDummyServerTestCase as server
+from urllib3 import HTTPConnectionPool
+from urllib3.response import HTTPResponse
+
+
+@pytest.fixture()
+def pool() -> Generator[HTTPConnectionPool, None, None]:
+ server.setup_class()
+
+ with HTTPConnectionPool(server.host, server.port) as pool:
+ yield pool
+
+ server.teardown_class()
+
+
+def test_returns_urllib3_HTTPResponse(pool: HTTPConnectionPool) -> None:
+ conn = pool._get_conn()
+
+ method = "GET"
+ path = "/"
+
+ conn.request(method, path)
+
+ response: HTTPResponse = conn.getresponse()
+
+ assert isinstance(response, HTTPResponse)
+
+
+def test_does_not_release_conn(pool: HTTPConnectionPool) -> None:
+ conn = pool._get_conn()
+
+ method = "GET"
+ path = "/"
+
+ conn.request(method, path)
+
+ response: HTTPResponse = conn.getresponse()
+
+ response.release_conn()
+ assert pool.pool.qsize() == 0 # type: ignore[union-attr]
+
+
+def test_releases_conn(pool: HTTPConnectionPool) -> None:
+ conn = pool._get_conn()
+
+ method = "GET"
+ path = "/"
+
+ conn.request(method, path)
+
+ response: HTTPResponse = conn.getresponse()
+ # If these variables are set by the pool
+ # then the response can release the connection
+ # back into the pool.
+ response._pool = pool
+ response._connection = conn
+
+ response.release_conn()
+ assert pool.pool.qsize() == 1 # type: ignore[union-attr]
+
+
+def test_double_getresponse(pool: HTTPConnectionPool) -> None:
+ conn = pool._get_conn()
+
+ method = "GET"
+ path = "/"
+
+ conn.request(method, path)
+
+ _: HTTPResponse = conn.getresponse()
+
+ # Calling getrepsonse() twice should cause an error
+ with pytest.raises(ResponseNotReady):
+ conn.getresponse()

MODIFY
test/with_dummyserver/test_socketlevel.py
test/with_dummyserver/test_socketlevel.py
@@ -40,6 +40,7 @@ from dummyserver.testcase import SocketDummyServerTestCase, consume_socket
from urllib3 import HTTPConnectionPool, HTTPSConnectionPool, ProxyManager, util
from urllib3._collections import HTTPHeaderDict
from urllib3.connection import HTTPConnection, _get_default_user_agent
+from urllib3.connectionpool import _url_from_pool
from urllib3.exceptions import (
MaxRetryError,
ProtocolError,
@@ -1791,7 +1792,7 @@ class TestBrokenHeaders(SocketDummyServerTestCase):
if (
"Failed to parse headers" in record.msg
and isinstance(record.args, tuple)
- and pool._absolute_url("/") == record.args[0]
+ and _url_from_pool(pool, "/") == record.args[0]
):
if (
unparsed_data_check is None

Change HTTPConnection.getresponse() to return urllib3.HTTPResponse

Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>