Skip to content

fix: deep-audit correctness + public-API findings#64

Merged
lesnik512 merged 10 commits into
mainfrom
fix/audit-correctness
Jun 14, 2026
Merged

fix: deep-audit correctness + public-API findings#64
lesnik512 merged 10 commits into
mainfrom
fix/audit-correctness

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Summary

Closes the confirmed correctness and public-API findings from the 2026-06-14 deep audit that #62/#63 didn't already cover. Each fix lands TDD-first from the audit's reproducer.

Finding Fix
L1 RetryBudget token spent before Retry-After > max_delay give-up reorder the give-up check before budget.try_withdraw() in both AsyncRetry and Retry
L3 _parse_retry_after OverflowError on a huge digit string crashes the loop except (ValueError, OverflowError) → degrades to "no hint"
Nit1 full_jitter_delay OverflowError at attempt_index >= 1024 catch OverflowError, clamp to max_delay; docstring corrected to match
Nit3 _strip_userinfo emitted http:///path for creds-but-no-host shared _reassemble() avoids the empty-authority triple-slash in both _strip_userinfo and _mask_query; normal URLs byte-identical
Nit6 _contains_custom_type NameError when msgspec absent guard → friendly ImportError before any msgspec.* ref
Nit2 _is_streaming_body_async ignored sync iterables shared _is_replayable_type helper; async detector now marks non-replayable sync iterables as streaming (replay guard no longer relies on an undocumented httpx2 detail)
L2 RetryBudget "asyncio-safe" docstring lacked the blocking caveat docstring-only clarification (lock is corruption-safe but can briefly block the loop thread)
L8 middleware/__init__.py had no __all__ (9+ star-leaks) explicit __all__ of the ten public names, matching sibling subpackages

Notes

  • Implemented as 8 focused commits + 1 reconciliation commit (the _reassemble unification — the initial _strip_userinfo fix didn't cover the masked-query path, which my added test caught).
  • An independent correctness review traced the two highest-risk changes (retry reorder, streaming-body symmetry) line-by-line: sync/async parity exact, no regressions.

Test plan

  • TDD reproducers for L1 (sync+async), L3, Nit1/Nit3/Nit6/Nit2/L8
  • just test — 620 passed, 100% coverage
  • just lint — ruff + ty clean
  • no httpx2._, no raw str(request.url) regressions

🤖 Generated with Claude Code

lesnik512 and others added 10 commits June 14, 2026 16:45
2.0 ** attempt_index raises OverflowError in Python for attempt_index >= 1024
rather than saturating to math.inf as the old docstring claimed. Catch the
OverflowError and clamp the ceiling directly to max_delay, which is the
correct clamped result. Tests added to pin both the no-raise and the exact
max_delay clamp behaviours.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a server's Retry-After header exceeds max_delay, the middleware
previously consumed a budget token before giving up, allowing a
Retry-After flood to silently drain shared-budget capacity. Move the
retry_after > max_delay guard before try_withdraw() in both AsyncRetry
and Retry so the token is only spent when a real retry will happen.

Adds test_retry_after_exceeds_max_delay_does_not_consume_budget_token
(async) and …_sync (sync) to pin the invariant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
For URLs like `http://user:pass@/path` (credentials but no hostname),
the previous reconstruction from `parts.hostname` yielded an empty
netloc, causing `urlunsplit` to emit `http:///path`.  Fix by slicing
the `user:pass@` prefix directly off `parts.netloc`; when the remainder
is empty (no real host), reconstruct without an authority section to
produce `http:/path` instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…spec absent

When msgspec is not installed, `_contains_custom_type` would raise
`NameError: name 'msgspec' is not defined` rather than the friendly
`ImportError`. Added an early guard that raises `ImportError(MISSING_DEPENDENCY_MESSAGE)`
when `import_checker.is_msgspec_installed` is False, matching the
pattern used in `MsgspecDecoder.__init__`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…g caveat

The module docstring said "asyncio-safe" without noting that a sync thread
holding the threading.Lock can briefly block the event-loop thread. Added
clarification that the critical section is intentionally tiny to bound
latency, while preserving the core safety guarantee (no torn state).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Factor `_is_replayable_type` shared helper so both async and sync
detectors use the same exclusion list. `_is_streaming_body_async` now
returns True when the body has `__iter__` and is not a replayable type
(bytes/bytearray/memoryview/str/dict/list/tuple), closing the gap where
a sync generator passed to AsyncClient bypassed the streaming-body
marker and the AsyncRetry replay guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r-import leakage

Without __all__, `from httpware.middleware import *` would expose Awaitable,
Callable, Protocol, TypeAlias, runtime_checkable, httpx2, chain, and
resilience. Added explicit __all__ listing exactly the 10 public names
(protocols + phase decorators). Tests added to test_public_api.py to guard
this contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
float(int("9" * 400)) raises OverflowError (not ValueError) in Python,
crashing the retry loop when a server sends a pathologically large
Retry-After integer string. Widen the except clause to
(ValueError, OverflowError) so oversized values fall back to jitter
backoff rather than propagating unhandled.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up reconciliation after the parallel correctness fixes:
- _strip_userinfo's no-host fix only covered the no-query case; _mask_query's
  urlunsplit re-introduced http:///path when a sensitive query was masked.
  Factor a shared _reassemble() helper used by both, so the empty-authority
  triple-slash is avoided everywhere (normal URLs stay byte-identical).
- hoist a function-body import in the msgspec test (ruff PLC0415) and use the
  project's # ty: ignore form.
- cover the empty-netloc query/fragment branches, the @-in-path guard, and
  mark the never-iterated sync-generator test body no-cover.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lesnik512 lesnik512 merged commit 6a7f6d1 into main Jun 14, 2026
5 checks passed
@lesnik512 lesnik512 deleted the fix/audit-correctness branch June 14, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant