fix(oauth): strip whitespace from HTTP header values on Linux#1401
fix(oauth): strip whitespace from HTTP header values on Linux#1401
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens OAuth HTTP header generation by sanitizing ASCII header values to avoid connection errors when Linux platform metadata (notably platform.version()) contains trailing whitespace/newlines.
Changes:
- Strip leading/trailing whitespace from ASCII header values in
_ascii_header_value(). - Add regression tests covering newline/whitespace sanitization in
_ascii_header_value()and_common_headers(). - Document the fix in the English/Chinese release notes and root changelog.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/kimi_cli/auth/oauth.py |
Trims ASCII header values before building common OAuth headers. |
tests/auth/test_ascii_header.py |
Adds regression coverage for header sanitization and whitespace-free common headers. |
docs/zh/release-notes/changelog.md |
Adds unreleased note describing the Linux header whitespace fix (ZH). |
docs/en/release-notes/changelog.md |
Adds unreleased note describing the Linux header whitespace fix (EN). |
CHANGELOG.md |
Adds unreleased entry for the header sanitization fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _ascii_header_value(value: str, *, fallback: str = "unknown") -> str: | ||
| try: | ||
| value.encode("ascii") | ||
| return value | ||
| return value.strip() | ||
| except UnicodeEncodeError: | ||
| sanitized = value.encode("ascii", errors="ignore").decode("ascii").strip() | ||
| return sanitized or fallback |
There was a problem hiding this comment.
_ascii_header_value() now strips whitespace for ASCII-only values, but unlike the Unicode-sanitization path it does not fall back when the stripped result becomes empty (e.g., value is only whitespace). Consider returning fallback when value.strip() is empty as well, to keep behavior consistent and avoid emitting empty header values.
| """Tests for _ascii_header_value and _common_headers in oauth module. | ||
|
|
||
| Regression tests for the issue where Linux kernel version strings containing | ||
| trailing whitespace/newlines (e.g. platform.version() returning | ||
| "#101-Ubuntu SMP ...\n") would be included in HTTP headers, causing | ||
| connection errors. | ||
| """ | ||
|
|
||
| from unittest.mock import patch | ||
|
|
||
| from kimi_cli.auth.oauth import _ascii_header_value, _common_headers | ||
|
|
There was a problem hiding this comment.
This new test module is missing from __future__ import annotations, which appears to be used consistently across the existing test suite. Adding it keeps test files stylistically consistent and avoids surprises with forward references in type hints.
| def test_no_whitespace_in_header_values(self, _mock_device_id, mock_platform) -> None: | ||
| """All header values must be free of leading/trailing whitespace.""" | ||
| mock_platform.node.return_value = "myhost" | ||
| mock_platform.version.return_value = "#101-Ubuntu SMP\n" |
There was a problem hiding this comment.
The test patches the entire kimi_cli.auth.oauth.platform module, but only configures node() and version(). Because _common_headers() also calls _device_model() (which calls platform.system(), platform.machine(), etc.), those calls become MagicMocks and can take unintended branches (e.g., system == "Darwin" evaluating truthy), making the test less robust. Prefer patching only platform.node/platform.version, or explicitly set system()/machine()/release() return values to deterministic strings.
| mock_platform.version.return_value = "#101-Ubuntu SMP\n" | |
| mock_platform.version.return_value = "#101-Ubuntu SMP\n" | |
| mock_platform.system.return_value = "Linux" | |
| mock_platform.machine.return_value = "x86_64" | |
| mock_platform.release.return_value = "6.8.0-101" |
Summary
_ascii_header_value()to fix connection errors on certain Linux systems (e.g. kernel 6.8.0-101) whereplatform.version()returns strings with trailing newlinesTest plan
pytest tests/auth/test_ascii_header.py— 5 tests passChecklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.