Skip to content

Commit 3265382

Browse files
fix: refuse overflowing server retry delays
1 parent 3cc8d78 commit 3265382

3 files changed

Lines changed: 99 additions & 18 deletions

File tree

src/openai/_base_client.py

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -759,24 +759,24 @@ def _parse_retry_after_header(self, response_headers: Optional[httpx2.Headers] =
759759
if response_headers is None:
760760
return None
761761

762-
# First, try the non-standard `retry-after-ms` header for milliseconds,
763-
# which is more precise than integer-seconds `retry-after`
764-
try:
765-
retry_ms_header = response_headers.get("retry-after-ms", None)
766-
return float(retry_ms_header) / 1000
767-
except (TypeError, ValueError):
768-
pass
769-
770-
# Next, try parsing `retry-after` header as seconds (allowing nonstandard floats).
771-
retry_header = response_headers.get("retry-after")
772-
try:
773-
# note: the spec indicates that this should only ever be an integer
774-
# but if someone sends a float there's no reason for us to not respect it
775-
return float(retry_header)
776-
except (TypeError, ValueError):
777-
pass
762+
# Prefer milliseconds, then seconds (allowing nonstandard floats).
763+
for header, divisor in (("retry-after-ms", 1000), ("retry-after", 1)):
764+
value = response_headers.get(header)
765+
if value is None:
766+
continue
767+
try:
768+
delay = float(value)
769+
except ValueError:
770+
continue
771+
if delay == math.inf and value.strip().lower() not in ("inf", "+inf", "infinity", "+infinity"):
772+
# Numeric overflow is an excessive server delay, not a malformed
773+
# infinity literal. Keep a finite sentinel so retry eligibility
774+
# refuses it instead of falling back to a shorter wait.
775+
return MAX_RETRY_AFTER_DELAY + 1
776+
return delay / divisor
778777

779778
# Last, try parsing `retry-after` as a date.
779+
retry_header = response_headers.get("retry-after")
780780
try:
781781
retry_date_tuple = email.utils.parsedate_tz(retry_header)
782782
if retry_date_tuple is None:

tests/test_retry_after_overflow.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
from __future__ import annotations
2+
3+
from unittest import mock
4+
5+
import httpx2
6+
import pytest
7+
8+
from openai import OpenAI, AsyncOpenAI, APIStatusError
9+
10+
11+
@pytest.mark.parametrize("async_mode", [False, True])
12+
@pytest.mark.parametrize(
13+
"status,headers,delay",
14+
[
15+
(429, {"retry-after": "1e999"}, None),
16+
(503, {"retry-after": "9" * 400}, None),
17+
(429, {"retry-after-ms": "1e999"}, None),
18+
(503, {"retry-after-ms": "9" * 400}, None),
19+
(401, {"retry-after": "1e999", "x-should-retry": "true"}, None),
20+
(429, {"retry-after": "inf"}, 0.5),
21+
(503, {"retry-after": " +Infinity "}, 0.5),
22+
(429, {"retry-after": "NaN"}, 0.5),
23+
(429, {"retry-after": "-1e999"}, 0.5),
24+
(429, {"retry-after-ms": "inf", "retry-after": "90"}, 0.5),
25+
(429, {"retry-after": "1e2"}, 100.0),
26+
(503, {"retry-after-ms": "1e5"}, 100.0),
27+
],
28+
)
29+
async def test_retry_after_numeric_overflow(
30+
async_mode: bool, status: int, headers: dict[str, str], delay: float | None
31+
) -> None:
32+
attempts = 0
33+
body = {"message": "Synthetic retry error", "type": "synthetic_error", "code": "synthetic_code"}
34+
35+
def handle(request: httpx2.Request) -> httpx2.Response:
36+
nonlocal attempts
37+
attempts += 1
38+
assert request.url.path == "/models/test"
39+
return httpx2.Response(status, headers={**headers, "x-request-id": "synthetic-id"}, json={"error": body})
40+
41+
with mock.patch("time.sleep") as sync_sleep, mock.patch("anyio.sleep") as async_sleep:
42+
with mock.patch("openai._base_client.random", return_value=0), pytest.raises(APIStatusError) as exc:
43+
if async_mode:
44+
async with AsyncOpenAI(
45+
api_key="synthetic-key",
46+
base_url="https://retry.test",
47+
max_retries=1,
48+
http_client=httpx2.AsyncClient(transport=httpx2.MockTransport(handle), trust_env=False),
49+
) as async_client:
50+
await async_client.models.retrieve("test")
51+
else:
52+
with OpenAI(
53+
api_key="synthetic-key",
54+
base_url="https://retry.test",
55+
max_retries=1,
56+
http_client=httpx2.Client(transport=httpx2.MockTransport(handle), trust_env=False),
57+
) as client:
58+
client.models.retrieve("test")
59+
60+
assert attempts == (1 if delay is None else 2)
61+
assert (async_sleep if async_mode else sync_sleep).call_args_list == ([] if delay is None else [mock.call(delay)])
62+
assert exc.value.status_code == status
63+
assert exc.value.body == body
64+
assert exc.value.request_id == "synthetic-id"
65+
assert all(exc.value.response.headers[key] == value for key, value in headers.items())

tests/test_x509_workload_identity_hardening.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,15 @@ def test_x509_rejects_data_residency_without_a_confirmed_mtls_endpoint(
322322
client.with_options(data_residency="ae")
323323

324324

325-
@pytest.mark.parametrize("headers", [{"x-should-retry": "false"}, {"retry-after-ms": "120001"}])
325+
@pytest.mark.parametrize(
326+
"headers",
327+
[
328+
{"x-should-retry": "false"},
329+
{"retry-after-ms": "120001"},
330+
{"retry-after": "1e999"},
331+
{"retry-after-ms": "9" * 400},
332+
],
333+
)
326334
def test_sync_x509_token_exchange_honors_server_retry_refusals(headers: dict[str, str]) -> None:
327335
requests: list[httpx2.Request] = []
328336

@@ -339,7 +347,15 @@ def handler(request: httpx2.Request) -> httpx2.Response:
339347
assert len(requests) == 1
340348

341349

342-
@pytest.mark.parametrize("headers", [{"x-should-retry": "false"}, {"retry-after-ms": "120001"}])
350+
@pytest.mark.parametrize(
351+
"headers",
352+
[
353+
{"x-should-retry": "false"},
354+
{"retry-after-ms": "120001"},
355+
{"retry-after": "1e999"},
356+
{"retry-after-ms": "9" * 400},
357+
],
358+
)
343359
async def test_async_x509_token_exchange_honors_server_retry_refusals(headers: dict[str, str]) -> None:
344360
requests: list[httpx2.Request] = []
345361

0 commit comments

Comments
 (0)