[PR #5453] [MERGED] fix(browser): read SAZ archives with zero-byte HTTP members #6930

Closed
opened 2026-11-01 21:44:09 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5453
Author: @amyanger
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: fix/5411-empty-saz-member


📝 Commits (1)

  • 4f17b4f fix(browser): read SAZ archives with zero-byte HTTP members

📊 Changes

4 files changed (+62 additions, -13 deletions)

View changed files

📝 docs/web-network-captures.md (+2 -0)
📝 src/browser/history/NodeCaptureAdapter.ts (+1 -1)
📝 src/browser/history/SazCapture.ts (+19 -12)
📝 tests/boundary/browser/sazWebCapture.test.ts (+40 -0)

📄 Description

Summary

Fixes #5411.

Fiddler writes a zero-byte raw/NN_s.txt (or _c.txt) when a session never got that message. inspect_web_network_capture passed those bytes to the HTTP decoder, which threw "SAZ HTTP message has no header/body separator", so the whole archive was rejected and no session could be read, even with --record. A zero-byte HTTP member is now reported like a missing one: the request or response is null, the member stays in location.members, and the record carries a limitation saying the stored member was empty. The SAZ capture-level limitation now says absent and zero-byte HTTP members are null.

Problem and expected behavior

Smallest trigger: a SAZ archive with one valid raw/1_c.txt and a zero-byte raw/1_s.txt. On base the CLI returns invalid_request / invalid_format for the whole archive. The documented contract already says "Missing members remain null, distinct from empty bodies", and a missing _s.txt already works. A zero-byte member has no start line, so it is not an HTTP message with an empty body; it is the absence of a recorded message.

Change and scope

decodeSazCapture in src/browser/history/SazCapture.ts owns the meaning of each archive member, so the fix lives there. When a _c.txt or _s.txt member is zero bytes, the decoder leaves request or response as null and adds a per-record limitation. The limitation states only that the stored member is empty. It does not claim the server never answered, because a zero-byte member could also come from an interrupted write. NodeCaptureAdapter.ts updates the SAZ capture-level limitation to match. decodeSazHttp is unchanged, so a non-empty member without a header/body separator is still rejected as malformed. The declared-size and stream-length checks still run before this branch. Empty _w.txt and _g.txt members were already accepted as zero-length opaque bytes and are unchanged.

Precedent: HAR (HarCapture.ts) and qlog (QlogCapture.ts) already report fixed per-record limitation sentences. The limitation text does not name the member path, so it cannot bypass the coordinate exclusion that applies when a member path contains an explicitly sensitive literal. Record limitations also pass through the service's redactText.

Not included: an empty _m.xml still rejects the archive through decodeSazMetadata. The issue does not cover it and I have not seen one from a real producer. The experiment note experiments/analysis-efficiency/saz-selection-workflows.md records the old failure as a baseline observation and is left as historical record.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: SAZ archive member decoding in src/browser/history/SazCapture.ts.
  • Contribution scope fit and linked maintainer decision: not applicable, a decoding fix within the existing capture boundary.
  • CLI and MCP/tool-catalog contract: no schema change. Archives that were rejected now return records; record limitations is an existing string[].
  • Provider, bridge, target-format, or platform compatibility: SAZ only. Other capture formats are untouched.
  • Evidence, artifact, provenance, or reconstruction contract: archives that already decoded produce identical output. Only previously rejected archives gain output.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none. docs/web-network-captures.md gains one sentence.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/browser/sazWebCapture.test.ts, "reports zero-byte SAZ HTTP members as null instead of rejecting the archive". It runs the public CLI on a synthetic archive with an empty _s.txt in one session, an empty _c.txt and an empty _w.txt in another, then checks null request/response, retained members, the exact limitations, that the empty WebSocket member stays retained without a limitation, and --record 0 selection.
  • Base reproduction or other evidence (executed): the final test file run against base aa314ab68 fails only the new test, with the issue's error: {"code":"invalid_request",...,"issues":[{"path":["capture_path",""],"reason":"invalid_format","message":"SAZ HTTP message has no header/body separator."}]}. Result: 1 failed, 16 passed, 1 skipped.
  • Mutation check (executed on the final diff, each mutant rebuilt and run against the SAZ boundary file in a separate worktree): disabling the zero-length branch, dropping the limitation, inverting the request flag passed to decodeSazHttp, not recognizing _c.txt, widening the HTTP-only branch to _w.txt, and emitting limitations: []. All 6 mutants fail at least one test. The capture-level limitation sentence is static text with no test asserting it, before or after this change.
  • User-visible CLI/MCP output (executed, real archive): x49_mp_cs21.saz from Microsoft's Xbox Live Trace Analyzer at ad880ff0, sha256 97cdca8b…7f6, as pinned in experiments/analysis-efficiency/corpus.json.
    • Base: rea inspect-web-network-capture x49_mp_cs21.saz saz --json returns the invalid_format error above.
    • Branch: exit 0, 103 of 103 records. Only ordinal 98 (session 099) carries a limitation: members raw/099_c.txt, raw/099_m.xml, raw/099_s.txt, request method POST, response: null, limitation "The stored response member is empty (zero bytes); the response is reported as null.". --record 99 --record 0 also exits 0.
  • Adversarial review (read-only agent): no consumer breakage found. It checked the record schema, redactText on record limitations, coordinate exclusion, route-template and environment-manifest handling of null request/response, protobuf pointer checks, duplicate and case-variant members, and the size checks. Its two notes, the empty _m.xml case and the historical experiment note, are covered under scope above.
  • Remaining proof gaps: the MCP path was not exercised separately for this case. It shares WebNetworkCaptureService with the CLI, and existing tests cover CLI and MCP equality for SAZ.

For evidence-bearing changes:

  • 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

All on Node from .nvmrc, macOS arm64, base aa314ab682e4bcd1e050a1e073bc90930660807f.

  • npm run test:focused -- tests/boundary/browser/sazWebCapture.test.ts tests/evaluation/sazWorkflowSemantics.test.ts src/browser/history/SazMetadata.test.ts src/browser/history/NodeCaptureAdapter.test.ts src/domain/webNetworkCapture.test.ts src/domain/analysisView/webNetworkCaptureView.test.ts: 6 files passed, 93 passed, 1 skipped
  • npm run check:fast: passed
  • npm run test:changed -- --base aa314ab68…: 2 passed. It selects only source-level tests; the boundary file above covers the decoder.
  • npm run check: passed
  • npm run docs:check: passed
  • /code-review high on the uncommitted diff: 7 findings, each checked by hand.
    • Fixed: the SAZ capture-level limitation still said only absent members are null.
    • Fixed: the per-record limitation claimed "no response was recorded", which a zero-byte member does not prove. It now states only that the member is empty.
    • Fixed: the _c/_s suffix checks were repeated in two chains. They are now one branch.
    • Not changed: an empty _m.xml still rejects the archive. It is outside this issue, and the real archive's empty session has a non-empty _m.xml.
    • Not changed: no structured "empty" flag in reported. location.members already shows the member exists, and adding a field would change the reported schema.
    • Not changed: limitation order follows ZIP member order. Record ordinals already follow ZIP order, so this matches the existing contract.
    • Not changed: no extra test for a session where both HTTP members are empty. That path uses the same branch twice, and the mutants above are already killed.

The skipped test is "reports SAZ unavailable when its runtime cannot enforce decoder memory", which is gated on runtimes without a memory bound and skips on base as well.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Previously rejected archives now decode.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; SAZ decoding is local file parsing. Exercised on a real Fiddler v4.6 archive as above.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the limitation text is fixed and names no path or value. Sensitive-literal exclusion of member coordinates is unchanged.

Author readiness

Human owner: @amyanger

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

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.

🔄 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/5453 **Author:** [@amyanger](https://github.com/amyanger) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/5411-empty-saz-member` --- ### 📝 Commits (1) - [`4f17b4f`](https://github.com/morluto/rea/commit/4f17b4f8a5f5fb8ebafa293368bcaf69d3a30b4f) fix(browser): read SAZ archives with zero-byte HTTP members ### 📊 Changes **4 files changed** (+62 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `docs/web-network-captures.md` (+2 -0) 📝 `src/browser/history/NodeCaptureAdapter.ts` (+1 -1) 📝 `src/browser/history/SazCapture.ts` (+19 -12) 📝 `tests/boundary/browser/sazWebCapture.test.ts` (+40 -0) </details> ### 📄 Description ## Summary Fixes #5411. Fiddler writes a zero-byte `raw/NN_s.txt` (or `_c.txt`) when a session never got that message. `inspect_web_network_capture` passed those bytes to the HTTP decoder, which threw "SAZ HTTP message has no header/body separator", so the whole archive was rejected and no session could be read, even with `--record`. A zero-byte HTTP member is now reported like a missing one: the request or response is `null`, the member stays in `location.members`, and the record carries a limitation saying the stored member was empty. The SAZ capture-level limitation now says absent and zero-byte HTTP members are null. ## Problem and expected behavior Smallest trigger: a SAZ archive with one valid `raw/1_c.txt` and a zero-byte `raw/1_s.txt`. On base the CLI returns `invalid_request` / `invalid_format` for the whole archive. The documented contract already says "Missing members remain null, distinct from empty bodies", and a missing `_s.txt` already works. A zero-byte member has no start line, so it is not an HTTP message with an empty body; it is the absence of a recorded message. ## Change and scope `decodeSazCapture` in `src/browser/history/SazCapture.ts` owns the meaning of each archive member, so the fix lives there. When a `_c.txt` or `_s.txt` member is zero bytes, the decoder leaves `request` or `response` as `null` and adds a per-record limitation. The limitation states only that the stored member is empty. It does not claim the server never answered, because a zero-byte member could also come from an interrupted write. `NodeCaptureAdapter.ts` updates the SAZ capture-level limitation to match. `decodeSazHttp` is unchanged, so a non-empty member without a header/body separator is still rejected as malformed. The declared-size and stream-length checks still run before this branch. Empty `_w.txt` and `_g.txt` members were already accepted as zero-length opaque bytes and are unchanged. Precedent: HAR (`HarCapture.ts`) and qlog (`QlogCapture.ts`) already report fixed per-record limitation sentences. The limitation text does not name the member path, so it cannot bypass the coordinate exclusion that applies when a member path contains an explicitly sensitive literal. Record limitations also pass through the service's `redactText`. Not included: an empty `_m.xml` still rejects the archive through `decodeSazMetadata`. The issue does not cover it and I have not seen one from a real producer. The experiment note `experiments/analysis-efficiency/saz-selection-workflows.md` records the old failure as a baseline observation and is left as historical record. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: SAZ archive member decoding in `src/browser/history/SazCapture.ts`. - Contribution scope fit and linked maintainer decision: not applicable, a decoding fix within the existing capture boundary. - CLI and MCP/tool-catalog contract: no schema change. Archives that were rejected now return records; record `limitations` is an existing `string[]`. - Provider, bridge, target-format, or platform compatibility: SAZ only. Other capture formats are untouched. - Evidence, artifact, provenance, or reconstruction contract: archives that already decoded produce identical output. Only previously rejected archives gain output. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. `docs/web-network-captures.md` gains one sentence. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/browser/sazWebCapture.test.ts`, "reports zero-byte SAZ HTTP members as null instead of rejecting the archive". It runs the public CLI on a synthetic archive with an empty `_s.txt` in one session, an empty `_c.txt` and an empty `_w.txt` in another, then checks null request/response, retained members, the exact limitations, that the empty WebSocket member stays retained without a limitation, and `--record 0` selection. - Base reproduction or other evidence (executed): the final test file run against base `aa314ab68` fails only the new test, with the issue's error: `{"code":"invalid_request",...,"issues":[{"path":["capture_path",""],"reason":"invalid_format","message":"SAZ HTTP message has no header/body separator."}]}`. Result: 1 failed, 16 passed, 1 skipped. - Mutation check (executed on the final diff, each mutant rebuilt and run against the SAZ boundary file in a separate worktree): disabling the zero-length branch, dropping the limitation, inverting the request flag passed to `decodeSazHttp`, not recognizing `_c.txt`, widening the HTTP-only branch to `_w.txt`, and emitting `limitations: []`. All 6 mutants fail at least one test. The capture-level limitation sentence is static text with no test asserting it, before or after this change. - User-visible CLI/MCP output (executed, real archive): `x49_mp_cs21.saz` from Microsoft's Xbox Live Trace Analyzer at `ad880ff0`, sha256 `97cdca8b…7f6`, as pinned in `experiments/analysis-efficiency/corpus.json`. - Base: `rea inspect-web-network-capture x49_mp_cs21.saz saz --json` returns the `invalid_format` error above. - Branch: exit 0, 103 of 103 records. Only ordinal 98 (session `099`) carries a limitation: members `raw/099_c.txt`, `raw/099_m.xml`, `raw/099_s.txt`, request method `POST`, `response: null`, limitation `"The stored response member is empty (zero bytes); the response is reported as null."`. `--record 99 --record 0` also exits 0. - Adversarial review (read-only agent): no consumer breakage found. It checked the record schema, `redactText` on record limitations, coordinate exclusion, route-template and environment-manifest handling of null request/response, protobuf pointer checks, duplicate and case-variant members, and the size checks. Its two notes, the empty `_m.xml` case and the historical experiment note, are covered under scope above. - Remaining proof gaps: the MCP path was not exercised separately for this case. It shares `WebNetworkCaptureService` with the CLI, and existing tests cover CLI and MCP equality for SAZ. For evidence-bearing changes: - [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 All on Node from `.nvmrc`, macOS arm64, base `aa314ab682e4bcd1e050a1e073bc90930660807f`. - `npm run test:focused -- tests/boundary/browser/sazWebCapture.test.ts tests/evaluation/sazWorkflowSemantics.test.ts src/browser/history/SazMetadata.test.ts src/browser/history/NodeCaptureAdapter.test.ts src/domain/webNetworkCapture.test.ts src/domain/analysisView/webNetworkCaptureView.test.ts`: 6 files passed, 93 passed, 1 skipped - `npm run check:fast`: passed - `npm run test:changed -- --base aa314ab68…`: 2 passed. It selects only source-level tests; the boundary file above covers the decoder. - `npm run check`: passed - `npm run docs:check`: passed - `/code-review high` on the uncommitted diff: 7 findings, each checked by hand. - Fixed: the SAZ capture-level limitation still said only absent members are null. - Fixed: the per-record limitation claimed "no response was recorded", which a zero-byte member does not prove. It now states only that the member is empty. - Fixed: the `_c`/`_s` suffix checks were repeated in two chains. They are now one branch. - Not changed: an empty `_m.xml` still rejects the archive. It is outside this issue, and the real archive's empty session has a non-empty `_m.xml`. - Not changed: no structured "empty" flag in `reported`. `location.members` already shows the member exists, and adding a field would change the reported schema. - Not changed: limitation order follows ZIP member order. Record ordinals already follow ZIP order, so this matches the existing contract. - Not changed: no extra test for a session where both HTTP members are empty. That path uses the same branch twice, and the mutants above are already killed. The skipped test is "reports SAZ unavailable when its runtime cannot enforce decoder memory", which is gated on runtimes without a memory bound and skips on base as well. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Previously rejected archives now decode. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; SAZ decoding is local file parsing. Exercised on a real Fiddler v4.6 archive as above. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the limitation text is fixed and names no path or value. Sensitive-literal exclusion of member coordinates is unchanged. ## Author readiness Human owner: @amyanger - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:44:09 +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#6930
No description provided.