fix(network): restore bounded BiDi opening-response validation - #242
Draft
seonghobae wants to merge 11 commits into
Draft
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Repair boundary
Restores and hardens the RFC 6455 WebDriver BiDi server opening-response boundary that had already existed in the historical transport stack but was dropped when PR #195 was reconstructed onto its retained prerequisite base. This dependency-root repair is stacked directly on current PR #195 exact head
5c111d0db6c363f9d1786c21cc01c5c7398007bd; no predecessor checks, reviews, approvals, or synthetic merge evidence are transferred.Current implementation
The branch validates the bounded HTTP/1.1 WebSocket opening response on the exact peer-verified stream, including
101 Switching Protocols, tokenizedUpgrade: websocket/Connection: Upgrade, a single client-key-correlatedSec-WebSocket-Accept, response-size and monotonic-deadline ceilings, typed read/cleanup failures, and fail-closed malformed-header handling. It rejects the malformed status-line formHTTP/1.1 101\r\n...: RFC status code parsing requires the mandatory separator before the reason phrase.It now also preserves HTTP/WebSocket extension interoperability at the correct authority boundary. RFC 9110 permits
obs-text(%x80-FF) in field values, while RFC 6455 requires unknown opening-handshake header fields to be ignored. The parser therefore no longer rejects a valid handshake solely because an ignored extension field contains opaque non-UTF-8 octets. Required handshake fields remain fail closed: anUpgrade,Connection, orSec-WebSocket-Acceptvalue containing such replacement material cannot satisfy the reviewed ASCII token or exact accept-value contract. The standards decision and APA 7 references are recorded indocs/doctoring.md.It performs no DNS/reconnect/proxy fallback, Chromium-process authentication, browser action, TLS-authority widening, unrestricted JavaScript execution, or browser/Agent authority grant.
Test-first / exact-head evidence
A realistic loopback status-line regression was added on exact test-only head
cb24998c62fda3b2f885ee4f1425569dd4a17598. Exact CI run33243277333reached the regression and failed semantically because the malformed no-separator status line was accepted.The ignored-extension compatibility defect was independently reproduced on exact test-only head
3c52466d1cd3978e1bb0a8f48a82c0c0e846fd43. Exact CI run33246054621checked out that head, passed repository contracts, formatting, and workspace compilation, then failed the full test step because a standards-valid unknown header containing0x80was rejected by the whole-response UTF-8 decoder.The smallest root-cause repair produced behavioral GREEN head
70cab65592c21d7d1008b467c7810eeae382e427: whole-response decoding is loss-tolerant for parsing/ignoring opaque extension-field content, while a dedicated hostile regression confirms non-ASCII replacement material still cannot turn an invalid requiredUpgradevalue intowebsocket. Exact CI run33246188780passed Rust contracts, full tests, strict Clippy, rustdoc, and exact owned-production function/line/region/branch coverage on that behavioral head.The subsequent standards-only documentation commit changed only
docs/doctoring.md. The current unchanged exact head is7ea14fd486706f7599e8e9f3919141a722e36ed4. Exact CI run33246431722is GREEN on this current head:99084590721checked out exact head and passed repository contracts, canonical formatting, workspace/all-target check, full tests, strict Clippy, and API documentation/rustdoc;99084590826checked out exact head and passed exact owned-production function, line, region, and branch measurement/diagnostics/enforcement.GitHub currently reports this Draft mergeable. No independent approval is claimed. Central required workflows that are absent on this Draft are not promoted as passing evidence.
Authority boundary
Protected-main
AGENTS.mdremains authoritative. This scheduled branch does not merge, self-approve, force-push, destructively rebase, alter workflows/rulesets/secrets, weaken checks, tag, release, or publish.