Repository navigation
fix(browser): save from gzip, deflate and cached responses within a bound - #7361
Conversation
…ound A saved response's decoded size is bounded by its received size, or the declared length of one served from cache, times 1032 for each gzip or deflate coding, the most DEFLATE expands. Grafana answers folder creates gzipped, so a UI step saving a new folder's uid was refused. Other codings and unknown or zero sizes stay refused before the body is read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
 [Medium impact] The PR appears safe to merge; no actionable issue was established. SummaryThis PR lets browser steps save values from gzip, deflate, and cached responses when their calculated decoded-size bound fits the existing limit.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Saved response or marked network content] --> B[Get received size or declared length]
B --> C[Calculate decoded-size bound]
C --> D{Known bound within limit?}
D -->|No| E[Refuse without reading body]
D -->|Yes| F[Read body]
F --> G{Actual size within limit?}
G -->|No| H[Reject body]
G -->|Yes| I[Save values or redact marked content]
Reviews (2) · Last reviewed commit: "merge: update main for browser response ..." · Reviewed by Greptile |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary
Since #7267, a scenario step that saves values from a response refuses any response whose
Content-Encodingis not absent oridentity, and any whose received size is unknown or zero, as from the browser's cache. Grafana answersPOST /api/foldersgzipped, so a UI step that saves a new folder'suidfails, and recordings that create Grafana folders through the UI cannot carry its id to later steps. #7162, which #7267 builds on, read such responses within a bound; that bound did not carry over (noted when closing #7162).This PR bounds a saved response's decoded size before reading it: its received size, or the declared
Content-Lengthof one served from cache, times 1032 for eachgzip,x-gzipordeflatecoding, the most DEFLATE (zlib) expands. A response whose bound fits the 12,000,000-byte limit is read; one whose bound passes it is refused before the body is read, as are other codings (br,zstd) and unknown or zero sizes. The same reader protects marked response content in network evidence, so a marked gzip body within the bound is now read and redacted rather than withheld.Problem and expected behavior
Change and scope
src/browser/BrowserScenarioResponseValues.ts:decodedBodyBound, used byreadBoundedResponseBodyfor UI saves and for marked network content.docs/browser-scenario-contract.md: the UI-step reading rule.src/browser/PlaywrightScenarioRequest.test.tsandsrc/browser/network/PlaywrightScenarioNetwork.test.ts.Contract and boundary impact
Expected effects:
Evidence and regression coverage
save_responseonPOST /api/items, then opens/items/{saved:item_1}. On main the save step fails with "the decoded response size cannot be bounded before reading it; this response body was not read."; on this branch every step completes and the destination opens with the saved id (/items/item-43).uidfromPOST /api/folders, whose answers were gzip-encoded.gzip,deflateandx-gzipanswers, from a cached answer by its declared length, and from a cached gzip answer; a gzip answer whose bound passes the limit is refused unread;br,gzip, zstd, zero and unknown sizes stay refused unread; marked gzip network content within the bound is read and redacted;brand unknown cache sizes stay withheld.83bbad2b.For evidence-bearing changes:
Validation performed
npm run -s check:fast,format:check,scan:todos,docs:check,verify:test-discovery,mcp-catalog:checkandjscpd: passed, merged with main27ac1b67.npx knip:staticFileUrlPathandObjcClassImportBindingCoverageonly, which main reports too.npx vitest run --project domain --project services: 11,538 passed, 6 skipped.--project adapters: 7,661 passed,19 skipped, 1 failed ("stops Inspector target authorization after the current stat settles"), which fails the same
way on main in this environment.
npm run -s test:focused --the environment and analysis-view CLI and MCP boundaries: 7 files, 79 passed.83bbad2band this branch.providerBinding.test.ts(bundle-root identity for artifact inventory),artifactInspectionCoverageMcp.test.ts(both FIFO cases) andghidraJavaDoctor.test.ts(empty PATH component),are outside the browser. The first three fail the same way on main
27ac1b67locally, and main's CI run 40436425782at
bfdde7eefails the bundle-root test too; the Ghidra doctor test passes locally on both. Other shards show nofailing test.
Compatibility, safety, and release
Maintainer validation
Merged main at ef11f77 into 4660989.
build:cached,check:fast, and focused browser response, provider binding, artifact MCP coverage, and Ghidra doctor tests passed (167 tests). GitHub CI is rerunning. No new real-browser run was performed for this merge.Author readiness
Human owner: @earakely-scale
🤖 Generated with Claude Code