Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle HTTP/1.1 half-closed connections gracefully. #641

Merged
merged 13 commits into from
Jul 7, 2023
20 changes: 16 additions & 4 deletions httpcore/_async/http11.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ConnectionNotAvailable,
LocalProtocolError,
RemoteProtocolError,
WriteError,
map_exceptions,
)
from .._models import Origin, Request, Response
Expand Down Expand Up @@ -84,10 +85,21 @@ async def handle_async_request(self, request: Request) -> Response:

try:
kwargs = {"request": request}
async with Trace("send_request_headers", logger, request, kwargs) as trace:
await self._send_request_headers(**kwargs)
async with Trace("send_request_body", logger, request, kwargs) as trace:
await self._send_request_body(**kwargs)
try:
async with Trace(
"send_request_headers", logger, request, kwargs
) as trace:
await self._send_request_headers(**kwargs)
async with Trace("send_request_body", logger, request, kwargs) as trace:
await self._send_request_body(**kwargs)
except WriteError:
# If we get a write error while we're writing the request,
# then we supress this error and move on to attempting to
# read the response. Servers can sometimes close the request
# pre-emptively and then respond with a well formed HTTP
# error response.
pass

async with Trace(
"receive_response_headers", logger, request, kwargs
) as trace:
Expand Down
20 changes: 16 additions & 4 deletions httpcore/_sync/http11.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ConnectionNotAvailable,
LocalProtocolError,
RemoteProtocolError,
WriteError,
map_exceptions,
)
from .._models import Origin, Request, Response
Expand Down Expand Up @@ -84,10 +85,21 @@ def handle_request(self, request: Request) -> Response:

try:
kwargs = {"request": request}
with Trace("send_request_headers", logger, request, kwargs) as trace:
self._send_request_headers(**kwargs)
with Trace("send_request_body", logger, request, kwargs) as trace:
self._send_request_body(**kwargs)
try:
with Trace(
"send_request_headers", logger, request, kwargs
) as trace:
self._send_request_headers(**kwargs)
with Trace("send_request_body", logger, request, kwargs) as trace:
self._send_request_body(**kwargs)
except WriteError:
# If we get a write error while we're writing the request,
Copy link
Member

@cdeler cdeler Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if server just closes the request, but don't answer (I saw it in some proxies)? Read will fail, so we will have partially-closed connection?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection is closed and the client raises an exception...

httpcore.RemoteProtocolError("Server disconnected without sending a response.")

I've added a test case to demonstrate this.

# then we supress this error and move on to attempting to
# read the response. Servers can sometimes close the request
# pre-emptively and then respond with a well formed HTTP
# error response.
pass

with Trace(
"receive_response_headers", logger, request, kwargs
) as trace:
Expand Down
105 changes: 105 additions & 0 deletions tests/_async/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
SOCKET_OPTION,
AsyncHTTPConnection,
AsyncMockBackend,
AsyncMockStream,
AsyncNetworkStream,
ConnectError,
ConnectionNotAvailable,
Origin,
RemoteProtocolError,
WriteError,
)


Expand Down Expand Up @@ -83,7 +86,109 @@ async def test_concurrent_requests_not_available_on_http11_connections():
await conn.request("GET", "https://example.com/")


@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
@pytest.mark.anyio
async def test_write_error_with_response_sent():
"""
If a server half-closes the connection while the client is sending
the request, it may still send a response. In this case the client
should successfully read and return the response.

See also the `test_write_error_without_response_sent` test above.
"""

class ErrorOnRequestTooLargeStream(AsyncMockStream):
def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
super().__init__(buffer, http2)
self.count = 0

async def write(
self, buffer: bytes, timeout: typing.Optional[float] = None
) -> None:
self.count += len(buffer)

if self.count > 1_000_000:
raise WriteError()

class ErrorOnRequestTooLarge(AsyncMockBackend):
async def connect_tcp(
self,
host: str,
port: int,
timeout: typing.Optional[float] = None,
local_address: typing.Optional[str] = None,
socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
) -> AsyncMockStream:
return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)

origin = Origin(b"https", b"example.com", 443)
network_backend = ErrorOnRequestTooLarge(
[
b"HTTP/1.1 413 Payload Too Large\r\n",
b"Content-Type: plain/text\r\n",
b"Content-Length: 37\r\n",
b"\r\n",
b"Request body exceeded 1,000,000 bytes",
]
)

async with AsyncHTTPConnection(
origin=origin, network_backend=network_backend, keepalive_expiry=5.0
) as conn:
content = b"x" * 10_000_000
response = await conn.request("POST", "https://example.com/", content=content)
assert response.status == 413
assert response.content == b"Request body exceeded 1,000,000 bytes"


@pytest.mark.anyio
@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
async def test_write_error_without_response_sent():
"""
If a server fully closes the connection while the client is sending
the request, then client should raise an error.

See also the `test_write_error_with_response_sent` test above.
"""

class ErrorOnRequestTooLargeStream(AsyncMockStream):
def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
super().__init__(buffer, http2)
self.count = 0

async def write(
self, buffer: bytes, timeout: typing.Optional[float] = None
) -> None:
self.count += len(buffer)

if self.count > 1_000_000:
raise WriteError()

class ErrorOnRequestTooLarge(AsyncMockBackend):
async def connect_tcp(
self,
host: str,
port: int,
timeout: typing.Optional[float] = None,
local_address: typing.Optional[str] = None,
socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
) -> AsyncMockStream:
return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)

origin = Origin(b"https", b"example.com", 443)
network_backend = ErrorOnRequestTooLarge([])

async with AsyncHTTPConnection(
origin=origin, network_backend=network_backend, keepalive_expiry=5.0
) as conn:
content = b"x" * 10_000_000
with pytest.raises(RemoteProtocolError) as exc_info:
await conn.request("POST", "https://example.com/", content=content)
assert str(exc_info.value) == "Server disconnected without sending a response."


@pytest.mark.anyio
@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
async def test_http2_connection():
origin = Origin(b"https", b"example.com", 443)
network_backend = AsyncMockBackend(
Expand Down
105 changes: 105 additions & 0 deletions tests/_sync/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
SOCKET_OPTION,
HTTPConnection,
MockBackend,
MockStream,
NetworkStream,
ConnectError,
ConnectionNotAvailable,
Origin,
RemoteProtocolError,
WriteError,
)


Expand Down Expand Up @@ -83,7 +86,109 @@ def test_concurrent_requests_not_available_on_http11_connections():
conn.request("GET", "https://example.com/")


@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")

def test_write_error_with_response_sent():
"""
If a server half-closes the connection while the client is sending
the request, it may still send a response. In this case the client
should successfully read and return the response.

See also the `test_write_error_without_response_sent` test above.
"""

class ErrorOnRequestTooLargeStream(MockStream):
def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
super().__init__(buffer, http2)
self.count = 0

def write(
self, buffer: bytes, timeout: typing.Optional[float] = None
) -> None:
self.count += len(buffer)

if self.count > 1_000_000:
raise WriteError()

class ErrorOnRequestTooLarge(MockBackend):
def connect_tcp(
self,
host: str,
port: int,
timeout: typing.Optional[float] = None,
local_address: typing.Optional[str] = None,
socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
) -> MockStream:
return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)

origin = Origin(b"https", b"example.com", 443)
network_backend = ErrorOnRequestTooLarge(
[
b"HTTP/1.1 413 Payload Too Large\r\n",
b"Content-Type: plain/text\r\n",
b"Content-Length: 37\r\n",
b"\r\n",
b"Request body exceeded 1,000,000 bytes",
]
)

with HTTPConnection(
origin=origin, network_backend=network_backend, keepalive_expiry=5.0
) as conn:
content = b"x" * 10_000_000
response = conn.request("POST", "https://example.com/", content=content)
assert response.status == 413
assert response.content == b"Request body exceeded 1,000,000 bytes"



@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
def test_write_error_without_response_sent():
"""
If a server fully closes the connection while the client is sending
the request, then client should raise an error.

See also the `test_write_error_with_response_sent` test above.
"""

class ErrorOnRequestTooLargeStream(MockStream):
def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
super().__init__(buffer, http2)
self.count = 0

def write(
self, buffer: bytes, timeout: typing.Optional[float] = None
) -> None:
self.count += len(buffer)

if self.count > 1_000_000:
raise WriteError()

class ErrorOnRequestTooLarge(MockBackend):
def connect_tcp(
self,
host: str,
port: int,
timeout: typing.Optional[float] = None,
local_address: typing.Optional[str] = None,
socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
) -> MockStream:
return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)

origin = Origin(b"https", b"example.com", 443)
network_backend = ErrorOnRequestTooLarge([])

with HTTPConnection(
origin=origin, network_backend=network_backend, keepalive_expiry=5.0
) as conn:
content = b"x" * 10_000_000
with pytest.raises(RemoteProtocolError) as exc_info:
conn.request("POST", "https://example.com/", content=content)
assert str(exc_info.value) == "Server disconnected without sending a response."



@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
def test_http2_connection():
origin = Origin(b"https", b"example.com", 443)
network_backend = MockBackend(
Expand Down