chore: input-validation pass — Request/Timeout/Limits/ClientConfig __post_init__ guards + charset parser fix#19
Merged
Conversation
Adds one final .strip() after the quote-stripping chain in _parse_charset so that Content-Type: ...; charset=" utf-8 " yields a clean codec name. Python's codec registry happens to normalize whitespace on lookup (so the end-to-end mojibake risk is currently masked), but the parser should still return a clean value rather than rely on codec leniency. Test verifies _parse_charset directly (the unit-level contract) plus an end-to-end smoke through Response.text. The other "concerns" listed in the deferred-work entry (substring false-positives, mismatched quotes, multi-charset directives) do not actually fire on the current code, per the spec's analysis. Closes deferred-work entry: "Charset parser robustness". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds Request.__post_init__ that validates: - url is a non-empty str - headers, params, cookies, extensions are each a Mapping - header and cookie names/values are non-empty str, no CR or LF with_* methods inherit validation via dataclasses.replace; no per-method code needed. Header validation is minimal per spec (reject CR/LF, non-str, empty); full RFC 9110 token validation is out of scope. Closes deferred-work entries: "Header name/value validation", "URL validation" (Request.url part), "with_query(None) handling". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both dataclasses now raise ValueError on construction if any field is negative. Zero is permitted (Timeout zero = fail-immediately sentinel; Limits zero = downstream's call on what it means, typically "no limit"). Closes deferred-work entry: "Timeout / Limits negative-value validation". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ClientConfig.__post_init__ now:
- rejects empty string / non-str base_url with ValueError
- strips trailing slash so the stored value is canonical
AsyncClient._resolve_url no longer does its own rstrip("/") on
base_url since the stored value is already canonical (DRY: one
source of truth for what a stored base_url looks like).
Closes deferred-work entry: "URL validation" (base_url normalization
part; the Request.url non-empty check shipped in the prior Request
__post_init__ commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes five Story 1-2 entries closed by this PR: charset parser robustness, header name/value validation, URL validation, with_query (None) handling, Timeout/Limits negative-value validation. The remaining Story 1-2 entries (multi-valued query params, streaming request bodies, @Final subclassing) stay open — they're different shapes of change, explicitly out of scope for this pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes five entries in
planning/deferred-work.md(Story 1-2 section). Spec:planning/specs/2026-06-03-input-validation-pass-design.md. Plan:planning/plans/2026-06-03-input-validation-pass-plan.md..strip()to_parse_charsetsoContent-Type: ...; charset=" iso-8859-1 "returns"iso-8859-1"instead of" iso-8859-1 ". The other "concerns" in the deferred entry don't actually fire on the current code.Request.__post_init__— validatesurlis a non-emptystr, all four mapping fields (headers,params,cookies,extensions) areMappings, and every header/cookie name/value is a non-emptystrwith no CR/LF. Validation lives at the dataclass boundary so allwith_*methods inherit it viadataclasses.replacewith no per-method code.Timeout.__post_init__/Limits.__post_init__— reject negative values for all 4 Timeout phase fields and all 3 Limits fields. Zero is permitted (Timeout zero = fail-immediately sentinel; Limits zero = downstream's call).ClientConfig.__post_init__— validatesbase_urlis a non-emptystrorNone, strips trailing slash(es) so the stored value is canonical. Rejects"/"/"///"(would normalize to empty). Removes the now-redundant.rstrip("/")fromAsyncClient._resolve_urlsince the stored value is already canonical.planning/deferred-work.md— removes the five closed Story 1-2 entries.Architecture: all validation lives in
__post_init__on the frozen dataclasses. Exception types:ValueErrorfor invalid values,TypeErrorfor wrong runtime types (one spec-sanctioned bundling:ClientConfig.base_urlusesValueErrorfor both since the type and emptiness paths share the same actionable message).This is a strict tightening. Code that previously silently accepted invalid inputs (empty URLs, CRLF in headers, negative timeouts, slash-only
base_url,Nonemapping fields, etc.) now raises at construction. No public API surface change;with_*methods unchanged.Five atomic commits.
Test plan
just lint-ciexits 0just test— 335 passing (296 baseline + 39 new across the 5 implementation commits)*_rejects_*tests intests/test_request.pyandtests/test_config.pypass (validates each new rule fires)tests/test_client_methods.py,tests/test_middleware.py,tests/test_transports_httpx2.pyall pass (no fixture regression from the new validation)grep -E 'Charset parser|Header name/value|URL validation|with_query|Timeout.*Limits.*negative' planning/deferred-work.mdreturns emptygrep -n 'rstrip' src/httpware/client.pyreturns empty (redundant rstrip removed)🤖 Generated with Claude Code