[PR #2773] [MERGED] fix(browser): keep malformed Content-Length declarations unknown #5382

Closed
opened 2026-10-31 01:06:12 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2773
Author: @yasshmittal
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/browser-content-length


📝 Commits (2)

  • f6ad6a4 fix(browser): keep malformed Content-Length declarations unknown
  • 88c1cb1 test(browser): cover the largest accepted Content-Length

📊 Changes

2 files changed (+29 additions, -9 deletions)

View changed files

📝 src/browser/CdpSafeMetadata.test.ts (+21 -0)
📝 src/browser/CdpSafeMetadata.ts (+8 -9)

📄 Description

Summary

Fixes #2596.

CDP response metadata turned malformed Content-Length headers into observed numeric lengths: -5 became 0, 0x10 became 16, 1e3 became 1000 and +12 became 12. Raw headers are discarded after normalization, so the capture could not recover the original spelling and reported a length the response never declared.

Problem and expected behavior

nonnegativeInteger in src/browser/CdpSafeMetadata.ts normalized the header with Number(), Math.trunc and a clamp to zero. RFC 9110 section 8.6 defines Content-Length = 1*DIGIT. Any other spelling declares no length and must stay null, which the nullable content_length schema already allows.

Change and scope

The header is parsed at its single normalization point. Only decimal digits within the safe-integer range count as a length (surrounding whitespace is trimmed). Every other value is null. The superseded nonnegativeInteger helper and its unused numberValue import are removed. Event ingestion (CdpCaptureEventHandlers) reaches this helper unchanged, so it has no separate interpretation to update.

Not included: accepting a list of identical values (42, 42). RFC 9110 permits recipients to do so but does not require it, and leaving it unknown is the conservative choice.

Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies.

Contract and boundary impact

  • Semantic owner and earliest changed stage: safeResponseMetadata header normalization (src/browser/CdpSafeMetadata.ts).

  • Contribution scope fit and linked maintainer decision: not applicable (bug fix within existing behavior).

  • CLI and MCP/tool-catalog contract: unchanged schema; malformed declarations now produce content_length: null instead of a number.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: removes values that were never observed; valid decimal lengths are unchanged.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated: a parameterized case in src/browser/CdpSafeMetadata.test.ts covering valid "100", padded " 0 ", numeric 100, the four malformed spellings from the issue, an unsafe-integer overflow, and an absent header.

  • Base reproduction or other evidence: on the base revision the four malformed cases return 0, 16, 1000 and 12, matching the issue.

  • User-visible CLI/MCP output: metadata.responses[].content_length is null for malformed declarations.

  • Remaining proof gaps: no real Chrome capture was run; the change concerns normalization of CDP header values, not browser behavior.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Host: Windows 11, Node.js 24.21.0.

  • npm run test:local -- src/browser/CdpSafeMetadata.test.ts: 37 passed.
  • npm run check: passed (0 lint errors, no new warnings, module boundaries and architecture guards passed).
  • npm run verify:test-discovery: all test files discovered exactly once.
  • Browser, Ghidra and SQLite test directories run with and without the change: no new failures.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows only; no real browser lane run.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new I/O, process, or network behavior; parsing is stricter.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2773 **Author:** [@yasshmittal](https://github.com/yasshmittal) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/browser-content-length` --- ### 📝 Commits (2) - [`f6ad6a4`](https://github.com/morluto/rea/commit/f6ad6a45cb2a8e1a69a27f27e0963bc55e1182c0) fix(browser): keep malformed Content-Length declarations unknown - [`88c1cb1`](https://github.com/morluto/rea/commit/88c1cb1f8eae0ba6992d0728d75eeb365b1b59ec) test(browser): cover the largest accepted Content-Length ### 📊 Changes **2 files changed** (+29 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpSafeMetadata.test.ts` (+21 -0) 📝 `src/browser/CdpSafeMetadata.ts` (+8 -9) </details> ### 📄 Description ## Summary Fixes #2596. CDP response metadata turned malformed `Content-Length` headers into observed numeric lengths: `-5` became `0`, `0x10` became `16`, `1e3` became `1000` and `+12` became `12`. Raw headers are discarded after normalization, so the capture could not recover the original spelling and reported a length the response never declared. ## Problem and expected behavior `nonnegativeInteger` in `src/browser/CdpSafeMetadata.ts` normalized the header with `Number()`, `Math.trunc` and a clamp to zero. RFC 9110 section 8.6 defines `Content-Length = 1*DIGIT`. Any other spelling declares no length and must stay `null`, which the nullable `content_length` schema already allows. ## Change and scope The header is parsed at its single normalization point. Only decimal digits within the safe-integer range count as a length (surrounding whitespace is trimmed). Every other value is `null`. The superseded `nonnegativeInteger` helper and its unused `numberValue` import are removed. Event ingestion (`CdpCaptureEventHandlers`) reaches this helper unchanged, so it has no separate interpretation to update. Not included: accepting a list of identical values (`42, 42`). RFC 9110 permits recipients to do so but does not require it, and leaving it unknown is the conservative choice. Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies. ## Contract and boundary impact - Semantic owner and earliest changed stage: `safeResponseMetadata` header normalization (`src/browser/CdpSafeMetadata.ts`). - Contribution scope fit and linked maintainer decision: not applicable (bug fix within existing behavior). - CLI and MCP/tool-catalog contract: unchanged schema; malformed declarations now produce `content_length: null` instead of a number. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: removes values that were never observed; valid decimal lengths are unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: a parameterized case in `src/browser/CdpSafeMetadata.test.ts` covering valid `"100"`, padded `" 0 "`, numeric `100`, the four malformed spellings from the issue, an unsafe-integer overflow, and an absent header. - Base reproduction or other evidence: on the base revision the four malformed cases return `0`, `16`, `1000` and `12`, matching the issue. - User-visible CLI/MCP output: `metadata.responses[].content_length` is `null` for malformed declarations. - Remaining proof gaps: no real Chrome capture was run; the change concerns normalization of CDP header values, not browser behavior. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Host: Windows 11, Node.js 24.21.0. - `npm run test:local -- src/browser/CdpSafeMetadata.test.ts`: 37 passed. - `npm run check`: passed (0 lint errors, no new warnings, module boundaries and architecture guards passed). - `npm run verify:test-discovery`: all test files discovered exactly once. - Browser, Ghidra and SQLite test directories run with and without the change: no new failures. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Windows only; no real browser lane run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new I/O, process, or network behavior; parsing is stricter. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:06:12 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#5382
No description provided.