Repository navigation
fix: sanitize http header newline - #1384
DRunkPiano114 wants to merge 2 commits into
Conversation
DRunkPiano114
changed the title
![devin-ai-integration[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR hardens OAuth/common HTTP header generation by sanitizing header values so they can’t contain control characters (e.g., trailing newlines from platform.version() on some Ubuntu kernels) that cause h11/aiohttp to reject requests.
Changes:
- Tighten
_ascii_header_value()to allow only TAB and printable ASCII and strip outer whitespace, falling back to"unknown"when empty. - Add unit + integration tests validating sanitized values are accepted by h11 and aiohttp.
- Document the fix in English/Chinese changelogs.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/kimi_cli/auth/oauth.py |
Replaces header-value sanitizer with a stricter printable-ASCII + TAB filter used by _common_headers() |
tests/auth/test_oauth_headers.py |
Adds unit and h11/aiohttp integration coverage for sanitization behavior and edge cases |
tests/auth/__init__.py |
Marks tests/auth as a package for test discovery/imports |
docs/zh/release-notes/changelog.md |
Adds unreleased note about HTTP header sanitization fix (ZH) |
docs/en/release-notes/changelog.md |
Adds unreleased note about HTTP header sanitization fix (EN) |
CHANGELOG.md |
Adds unreleased note about HTTP header sanitization fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| from h11._headers import normalize_and_validate | ||
|
|
||
| def _validate(value: str): | ||
| headers = [(b"X-Test-Header", value.encode("ascii"))] | ||
| normalize_and_validate(headers, _parsed=False) |
There was a problem hiding this comment.
The test imports h11._headers.normalize_and_validate, which is an internal/private API of h11 and could change when dependencies are updated. If you want to reduce brittleness, consider validating through a public h11 API path (e.g., constructing a h11.Request/h11.Response and letting h11 validate headers during event creation/sending), or gracefully skipping if the internal helper isn’t available.
| from h11._headers import normalize_and_validate | |
| def _validate(value: str): | |
| headers = [(b"X-Test-Header", value.encode("ascii"))] | |
| normalize_and_validate(headers, _parsed=False) | |
| try: | |
| import h11 | |
| except ImportError: | |
| pytest.skip("h11 not available") | |
| def _validate(value: str): | |
| headers = [(b"X-Test-Header", value.encode("ascii"))] | |
| # Constructing a Request triggers h11's header validation | |
| h11.Request(method="GET", target="/", headers=headers) |
|
|
||
| def _validate(value: str): | ||
| # _safe_header validates the full "Name: value" string | ||
| _safe_header(f"X-Test-Header: {value}") |
There was a problem hiding this comment.
The aiohttp integration test relies on aiohttp.http_writer._safe_header, which is a private/internal helper and not part of aiohttp’s public API. Even with the current pin, this can make tests brittle when aiohttp is upgraded; consider exercising header validation through a public API (or a higher-level constructor that performs the same checks), or broaden the skip logic to handle API/behavior changes beyond just ImportError.
| _safe_header(f"X-Test-Header: {value}") | |
| try: | |
| _safe_header(f"X-Test-Header: {value}") | |
| except Exception: | |
| pytest.skip("aiohttp _safe_header behavior changed; skipping integration test") |
|
Fixed on 1.20.0 via #1401 |
Related Issue
Closes #1321, closes #1364, closes #1368
Description
platform.version()on Ubuntu 22.04 HWE returns a string with a trailing newline (\n), which h11 rejects as an illegal header value, causing aConnection erroron all requests.This PR replaces
_ascii_header_value()with a stricter sanitizer that only permits TAB (\x09) and printable ASCII (\x20–\x7e), rejecting both non-ASCII bytes and control characters.Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.