Skip to content

Validate a caller-supplied extensions["timeout"] #1189

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.

Sign up for GitHub

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

Open
ebarkhordar wants to merge 2 commits into
base: main
Choose a base branch
from
+45 −0
Open

Validate a caller-supplied extensions["timeout"] #1189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1037938
Validate a caller-supplied `extensions["timeout"]`
ebarkhordar Sep 4, 2026
f715034
test: compare the timeout extension against a snapshot, not itself
ebarkhordar Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/httpx2/httpx2/_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,19 @@ class UseClientDefault:
ACCEPT_ENCODING = ", ".join([key for key in SUPPORTED_DECODERS.keys() if key != "identity"])


def _validate_timeout_extension(timeout: typing.Any) -> None:
# A caller-supplied `extensions["timeout"]` reaches the transport unchanged, and its
# values are passed to `socket.settimeout()`, which only accepts numbers.
if not isinstance(timeout, typing.Mapping):
raise TypeError(
f"extensions['timeout'] must be a mapping, got {type(timeout).__name__}. "
"Use `Timeout(...).as_dict()` to build one."
)
for name, value in timeout.items():
if value is not None and not isinstance(value, (int, float)):

@cubic-dev-ai cubic-dev-ai Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: A boolean slips through the int/float check because bool subclasses int, so extensions['timeout'] = {"read": True} passes validation and becomes a 1-second timeout instead of raising. This contradicts the PR's goal of rejecting non-timeout values. Explicitly exclude bool from the accepted types.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/httpx2/httpx2/_client.py, line 125:

<comment>A boolean slips through the int/float check because `bool` subclasses `int`, so `extensions['timeout'] = {"read": True}` passes validation and becomes a 1-second timeout instead of raising. This contradicts the PR's goal of rejecting non-timeout values. Explicitly exclude `bool` from the accepted types.</comment>

<file context>
@@ -113,6 +113,19 @@ class UseClientDefault:
+            "Use `Timeout(...).as_dict()` to build one."
+        )
+    for name, value in timeout.items():
+        if value is not None and not isinstance(value, (int, float)):
+            raise TypeError(f"extensions['timeout'][{name!r}] must be a number or None, got {type(value).__name__}.")
+
</file context>

@ebarkhordar ebarkhordar Sep 4, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'd leave this one. The guard admits exactly what socket.settimeout admits, which is the point of it: nothing that works today starts raising. bool is in that set, and it is reachable without the extensions dict at all, since Timeout(5.0, read=True).as_dict() returns {'read': True, ...} and the value goes straight through. So excluding it here would only make the two paths disagree.

The behaviour is worse than a 1s read though, and that part is worth knowing: settimeout(False) sets the socket non-blocking rather than raising. Happy to file that separately against Timeout if you think it's worth rejecting bools on both paths.

raise TypeError(f"extensions['timeout'][{name!r}] must be a number or None, got {type(value).__name__}.")


class ClientState(enum.Enum):
# UNOPENED:
# The client has been instantiated, but has not been used to send a request,
Expand Down Expand Up @@ -357,6 +370,8 @@ def build_request(
if "timeout" not in extensions:
timeout = self.timeout if isinstance(timeout, UseClientDefault) else Timeout(timeout)
extensions = dict(**extensions, timeout=timeout.as_dict())
else:
_validate_timeout_extension(extensions["timeout"])
return Request(
method,
url,
Expand Down Expand Up @@ -561,6 +576,8 @@ def _set_timeout(self, request: Request) -> None:
if "timeout" not in request.extensions:
timeout = self.timeout if isinstance(self.timeout, UseClientDefault) else Timeout(self.timeout)
request.extensions = dict(**request.extensions, timeout=timeout.as_dict())
else:
_validate_timeout_extension(request.extensions["timeout"])


class Client(BaseClient):
Expand Down
28 changes: 28 additions & 0 deletions tests/httpx2/test_timeouts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import copy
import typing

import pytest
Expand Down Expand Up @@ -58,3 +59,30 @@ async def test_async_client_new_request_send_timeout(server: TestServer) -> None
async with httpx2.AsyncClient(timeout=timeout) as client:
with pytest.raises(httpx2.TimeoutException):
await client.send(httpx2.Request("GET", server.url.copy_with(path="/slow_response")))


@pytest.mark.parametrize("name", ["connect", "read", "write", "pool"])
def test_timeout_extension_value_must_be_a_number(name: str) -> None:
request = httpx2.Request("GET", "http://127.0.0.1:1/")
request.extensions["timeout"] = {name: httpx2.Timeout(5.0)}

with pytest.raises(TypeError, match=f"extensions\\['timeout'\\]\\[{name!r}\\]"):
httpx2.Client().send(request)


def test_timeout_extension_must_be_a_mapping() -> None:
request = httpx2.Request("GET", "http://127.0.0.1:1/")
request.extensions["timeout"] = httpx2.Timeout(5.0)

with pytest.raises(TypeError, match="extensions\\['timeout'\\] must be a mapping"):
httpx2.Client().send(request)


@pytest.mark.parametrize("timeout", [{"connect": 5.0}, {"read": 5}, {"connect": None}, {}])
def test_timeout_extension_accepts_numbers_and_none(timeout: dict[str, float | None]) -> None:
# `Request` shallow-copies `extensions`, so the mapping reaching the transport is the
# caller's own object. Compare against a snapshot or the assertion is `x == x`.
expected = copy.deepcopy(timeout)
request = httpx2.Client().build_request("GET", "http://127.0.0.1:1/", extensions={"timeout": timeout})

assert request.extensions["timeout"] == expected
Loading