Skip to content

Commit

Permalink
Handle HTTP/1.1 half-closed connections gracefully. (#641)
Browse files Browse the repository at this point in the history
* Handle cases like HTTP 413 where the request writes fail, but a response is still sent correctly.

* Fix 'test_write_error_but_response_sent' test case

* Linting

* Fix imports

* Fix imports

* Fix imports

* Filter out 'PytestUnraisableExceptionWarning'

* Add tests for when the server closes the connection during request writing and does not send a response

* Linting
  • Loading branch information
tomchristie committed Jul 7, 2023
1 parent 80ff02f commit 04842b0
Show file tree
Hide file tree
Showing 4 changed files with 242 additions and 8 deletions.
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,
# 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

0 comments on commit 04842b0

Please sign in to comment.