fix(timeline): headers info in response timeline#8766
Conversation
WalkthroughThe change adds shared header-source resolution, captures transport and pre-request-script header provenance, and applies ordered header rendering across request details, request timeline, network logs, and header tables. ChangesHeader provenance and timeline rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PreRequest
participant AxiosNetwork
participant ClientRequest
participant HeaderResolver
participant BrunoApp
PreRequest->>PreRequest: identify scriptSetHeaders
AxiosNetwork->>ClientRequest: execute request
ClientRequest-->>AxiosNetwork: expose sent headers
AxiosNetwork->>HeaderResolver: provide timeline and request headers
HeaderResolver-->>BrunoApp: return ordered header rows
BrunoApp-->>BrunoApp: render request and network headers
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-electron/src/ipc/network/index.js (1)
1792-1799: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRunner path's
requestSentis missingscriptSetHeaders.The standalone
runRequestpath includesscriptSetHeaders: request.scriptSetHeaders || []in itsrequestSentpayload (line 992), but this Runner-flowrequestSent(built from the samerunPreRequest-populatedrequest.scriptSetHeaders) omits it. Header-provenance attribution for pre-request-script-set headers will silently break for Collection Runner runs.🐛 Proposed fix
let requestSent = { url: request.url, method: request.method, headers: headersSent, + scriptSetHeaders: request.scriptSetHeaders || [], data: requestData, dataBuffer: requestDataBuffer, timestamp: Date.now() };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/ipc/network/index.js` around lines 1792 - 1799, Update the Runner-flow requestSent payload in the request execution path to include scriptSetHeaders from request, defaulting to an empty array as in the standalone runRequest payload. Preserve the existing requestSent fields and use the runPreRequest-populated request.scriptSetHeaders value.
🧹 Nitpick comments (1)
packages/bruno-common/src/utils/headers-source.js (1)
1-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the utility with the package TypeScript extension.
packages/bruno-commonalready has real TypeScript runtime source,tsconfig.json, and@rollup/plugin-typescript, so this new utility should beheaders-source.tswith appropriate typed inputs/outputs instead ofheaders-source.js.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-common/src/utils/headers-source.js` around lines 1 - 108, The new headers utility should use the TypeScript extension and typed interfaces throughout. Rename the implementation to headers-source.ts, add appropriate types for header entries, timeline entries, request data, collection/item inputs, and buildHeaderRows inputs/outputs, and ensure the existing behavior remains unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-electron/src/ipc/network/axios-instance.js`:
- Around line 234-273: Make redirect timelines hop-aware in backfillSentHeaders
at packages/bruno-electron/src/ipc/network/axios-instance.js:234-273 by
resetting the existing-header set and insertion boundary at the most recent
request marker, so only the current hop is scanned. Update sortRequestHeaderLogs
at
packages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Network/index.js:11-40
to split slots at each request entry and sort/reassign request headers
independently within each hop segment.
- Around line 238-255: Update parseSentHeaders to stop reading the private
ClientRequest._header field. Use an API-supported source or mechanism that
preserves the sent wire-header names and ordering for timeline backfill, while
retaining the existing fallback behavior only if it provides equivalent
wire-header data; do not rely on getHeaders() as a casing/order substitute.
In `@packages/bruno-js/src/bruno-request.js`:
- Around line 140-153: Update setHeaders in the array-form merge path to filter
out prototype-pollution keys (__proto__, constructor, and prototype) from each
script-supplied entry before merging into this.req.headers. Preserve merging of
all other valid header keys and the existing object-form replacement behavior.
---
Outside diff comments:
In `@packages/bruno-electron/src/ipc/network/index.js`:
- Around line 1792-1799: Update the Runner-flow requestSent payload in the
request execution path to include scriptSetHeaders from request, defaulting to
an empty array as in the standalone runRequest payload. Preserve the existing
requestSent fields and use the runPreRequest-populated request.scriptSetHeaders
value.
---
Nitpick comments:
In `@packages/bruno-common/src/utils/headers-source.js`:
- Around line 1-108: The new headers utility should use the TypeScript extension
and typed interfaces throughout. Rename the implementation to headers-source.ts,
add appropriate types for header entries, timeline entries, request data,
collection/item inputs, and buildHeaderRows inputs/outputs, and ensure the
existing behavior remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b516d0ab-9d59-43cc-9fda-77749788f80e
📒 Files selected for processing (11)
packages/bruno-app/src/components/Devtools/Console/RequestDetailsPanel/index.jspackages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Common/Headers/index.jspackages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Network/index.jspackages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Request/index.jspackages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/StyledWrapper.jspackages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/index.jspackages/bruno-common/src/utils/headers-source.jspackages/bruno-common/src/utils/index.tspackages/bruno-electron/src/ipc/network/axios-instance.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-js/src/bruno-request.js
| // The Node http adapter appends transport headers (Host, Connection, Accept-Encoding, | ||
| // Content-Length) after the request interceptor logged config.headers, so they're missing from the | ||
| // timeline. Read the real serialized header block off the ClientRequest and backfill the ones we | ||
| // didn't already log, inserted right after the existing request-header block so ordering holds. | ||
| const parseSentHeaders = (req) => { | ||
| const raw = req?._header; | ||
| if (typeof raw === 'string') { | ||
| return raw | ||
| .split('\r\n') | ||
| .slice(1) // drop the request line (e.g. "GET /path HTTP/1.1") | ||
| .filter(Boolean) | ||
| .map((line) => { | ||
| const idx = line.indexOf(':'); | ||
| return idx === -1 ? null : { name: line.slice(0, idx).trim(), value: line.slice(idx + 1).trim() }; | ||
| }) | ||
| .filter((h) => h && h.name); | ||
| } | ||
| const hdrs = typeof req?.getHeaders === 'function' ? req.getHeaders() : null; | ||
| return hdrs | ||
| ? Object.entries(hdrs).map(([name, value]) => ({ name, value: Array.isArray(value) ? value.join(', ') : String(value) })) | ||
| : []; | ||
| }; | ||
|
|
||
| const backfillSentHeaders = (timeline, req) => { | ||
| if (!Array.isArray(timeline) || !req) return; | ||
| const existing = new Set(); | ||
| let lastIdx = -1; | ||
| timeline.forEach((entry, i) => { | ||
| if (entry?.type !== 'requestHeader' || typeof entry.message !== 'string') return; | ||
| const idx = entry.message.indexOf(':'); | ||
| if (idx !== -1) existing.add(entry.message.slice(0, idx).trim().toLowerCase()); | ||
| lastIdx = i; | ||
| }); | ||
| const additions = parseSentHeaders(req) | ||
| .filter((h) => !existing.has(h.name.toLowerCase())) | ||
| .map((h) => ({ timestamp: new Date(), type: 'requestHeader', message: `${h.name}: ${h.value}` })); | ||
| if (!additions.length) return; | ||
| timeline.splice(lastIdx >= 0 ? lastIdx + 1 : timeline.length, 0, ...additions); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Redirects share one timeline/logs array across hops; new header logic in both files isn't hop-aware. requestConfig.metadata.timeline = timeline (axios-instance.js, pre-existing) reuses the same array for every redirect hop of a request. Neither backfillSentHeaders nor sortRequestHeaderLogs segments by hop, so both misbehave specifically on redirected requests.
packages/bruno-electron/src/ipc/network/axios-instance.js#L234-L273: scope theexisting/lastIdxscan inbackfillSentHeadersto entries after the most recenttype === 'request'marker (the start of the current hop), not the whole array.packages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Network/index.js#L11-L40: segmentslotsby hop boundary (reset at eachtype === 'request'entry) insortRequestHeaderLogs, and sort/reassign within each segment independently instead of globally across all hops.
📍 Affects 2 files
packages/bruno-electron/src/ipc/network/axios-instance.js#L234-L273(this comment)packages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Network/index.js#L11-L40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/ipc/network/axios-instance.js` around lines 234 -
273, Make redirect timelines hop-aware in backfillSentHeaders at
packages/bruno-electron/src/ipc/network/axios-instance.js:234-273 by resetting
the existing-header set and insertion boundary at the most recent request
marker, so only the current hop is scanned. Update sortRequestHeaderLogs at
packages/bruno-app/src/components/ResponsePane/Timeline/TimelineItem/Network/index.js:11-40
to split slots at each request entry and sort/reassign request headers
independently within each hop segment.
| const parseSentHeaders = (req) => { | ||
| const raw = req?._header; | ||
| if (typeof raw === 'string') { | ||
| return raw | ||
| .split('\r\n') | ||
| .slice(1) // drop the request line (e.g. "GET /path HTTP/1.1") | ||
| .filter(Boolean) | ||
| .map((line) => { | ||
| const idx = line.indexOf(':'); | ||
| return idx === -1 ? null : { name: line.slice(0, idx).trim(), value: line.slice(idx + 1).trim() }; | ||
| }) | ||
| .filter((h) => h && h.name); | ||
| } | ||
| const hdrs = typeof req?.getHeaders === 'function' ? req.getHeaders() : null; | ||
| return hdrs | ||
| ? Object.entries(hdrs).map(([name, value]) => ({ name, value: Array.isArray(value) ? value.join(', ') : String(value) })) | ||
| : []; | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Is Node.js http ClientRequest._header a stable internal property across recent Node LTS versions?
💡 Result:
No, http.ClientRequest._header is not a stable or public API in Node.js [1][2][3]. It is an internal property that has never been documented as officially supported [3]. While it may still exist in some versions of Node.js for legacy compatibility or internal use [4][5], relying on it is strongly discouraged as it is subject to change or removal without notice [6]. It is important to distinguish _header from the deprecated _headers and _headerNames properties [6][3]. While _headers and _headerNames were formally deprecated (DEP0066) in Node.js v12.0.0 because they were internal, access to them triggers runtime warnings [3][7]. The _header property similarly falls under the category of non-public internal implementation details [3]. For working with HTTP headers in Node.js, you should always use the official public API methods [6][3]: - request.setHeader(name, value) [1] - request.getHeader(name) [1] - request.removeHeader(name) [1] - request.getHeaders() [6] - request.getHeaderNames() [3] If you need access to the final headers, use the provided public methods instead of attempting to access private properties prefixed with an underscore [6][3].
Citations:
- 1: https://nodejs.org/api/http.html
- 2: https://github.com/nodejs/node/blob/main/doc/api/http.md
- 3: nodejs/node@9d9b32e
- 4: https://github.com/nodejs/node/pull/64393/files
- 5: nodejs/node@4210259
- 6: https://www.mintlify.com/nodejs/userland-migrations/recipes/http-outgoingmessage-headers
- 7: https://stackoverflow.com/questions/56697360/how-to-fix-node12388-dep0066-deprecationwarning-outgoingmessage-prototype
🏁 Script executed:
#!/bin/bash
set -euo pipefon
echo "== locate axios-instance =="
fd -a 'axios-instance.js$' . || true
echo "== relevant file section =="
file="$(fd 'axios-instance.js$' packages/bruno-electron/src/ipc/network | head -n 1 || true)"
if [ -n "${file:-}" ]; then
sed -n '210,270p' "$file" | cat -n
echo "== package/version signals =="
fd 'package.json$' . | sed -n '1,20p' | while read -r p; do
echo "-- $p --"
sed -n '1,80p' "$p" | rg -n '"node"|"electron"|"engines"|"prebuild"|"`@node`"' || true
done
echo "== _header/getHeaders usages =="
rg -n "_header|getHeaders\\(" packages/bruno-electron/src/ipc/network/axios-instance.js packages || true
fiRepository: usebruno/bruno
Length of output: 200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate axios-instance =="
fd 'axios-instance.js$' . || true
file="$(fd 'axios-instance.js$' .packages/bruno-electron/src/ipc/network 2>/dev/null || fd 'axios-instance.js$' packages/bruno-electron/src/ipc/network | head -n 1 || true)"
echo "file=$file"
if [ -n "${file:-}" ]; then
echo "== relevant file section =="
sed -n '210,270p' "$file"
echo "== package/version signals =="
fd 'package.json$' . | sed -n '1,20p' | while read -r p; do
echo "-- $p --"
sed -n '1,120p' "$p" | rg -n '"node"|"electron"|"engines"|"prebuild"|"`@node`"' || true
done
echo "== _header/getHeaders usages =="
rg -n "_header|getHeaders\\(" packages/bruno-electron/src/ipc/network/axios-instance.js packages || true
fiRepository: usebruno/bruno
Length of output: 49314
Don’t depend on ClientRequest._header for request-header timeline backfill.
_header is a non-API Node http.ClientRequest implementation detail. If it disappears in the Node/Electron runtime, the parser falls back to getHeaders(), which uses lowercased header names and different casing/ordering. Use an API-supported approach for backfilling wire headers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/ipc/network/axios-instance.js` around lines 238 -
255, Update parseSentHeaders to stop reading the private ClientRequest._header
field. Use an API-supported source or mechanism that preserves the sent
wire-header names and ordering for timeline backfill, while retaining the
existing fallback behavior only if it provides equivalent wire-header data; do
not rely on getHeaders() as a casing/order substitute.
| setHeaders(headers) { | ||
| // Array form (e.g. [{ 'header-1': 'value-1' }, ...]) merges each entry onto the existing | ||
| // headers, preserving ones set elsewhere. Object form replaces the whole header set. | ||
| if (Array.isArray(headers)) { | ||
| if (!this.req.headers || typeof this.req.headers !== 'object' || Array.isArray(this.req.headers)) { | ||
| this.req.headers = {}; | ||
| } | ||
| headers.forEach((entry) => { | ||
| if (entry && typeof entry === 'object') Object.assign(this.req.headers, entry); | ||
| }); | ||
| return; | ||
| } | ||
| this.req.headers = headers; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prototype pollution risk via array-form setHeaders.
Object.assign(this.req.headers, entry) merges arbitrary keys from script-supplied objects; a __proto__/constructor/prototype key would pollute the prototype chain.
Based on the static analysis hint: "Merging request-derived input into a target object via Object.assign ... can pollute Object.prototype when the payload contains "proto"..." (prototype-pollution-recursive-merge-javascript).
🛡️ Proposed fix — filter dangerous keys
headers.forEach((entry) => {
- if (entry && typeof entry === 'object') Object.assign(this.req.headers, entry);
+ if (entry && typeof entry === 'object') {
+ Object.keys(entry).forEach((key) => {
+ if (key === '__proto__' || key === 'constructor' || key === 'prototype') return;
+ this.req.headers[key] = entry[key];
+ });
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setHeaders(headers) { | |
| // Array form (e.g. [{ 'header-1': 'value-1' }, ...]) merges each entry onto the existing | |
| // headers, preserving ones set elsewhere. Object form replaces the whole header set. | |
| if (Array.isArray(headers)) { | |
| if (!this.req.headers || typeof this.req.headers !== 'object' || Array.isArray(this.req.headers)) { | |
| this.req.headers = {}; | |
| } | |
| headers.forEach((entry) => { | |
| if (entry && typeof entry === 'object') Object.assign(this.req.headers, entry); | |
| }); | |
| return; | |
| } | |
| this.req.headers = headers; | |
| } | |
| setHeaders(headers) { | |
| // Array form (e.g. [{ 'header-1': 'value-1' }, ...]) merges each entry onto the existing | |
| // headers, preserving ones set elsewhere. Object form replaces the whole header set. | |
| if (Array.isArray(headers)) { | |
| if (!this.req.headers || typeof this.req.headers !== 'object' || Array.isArray(this.req.headers)) { | |
| this.req.headers = {}; | |
| } | |
| headers.forEach((entry) => { | |
| if (entry && typeof entry === 'object') { | |
| Object.keys(entry).forEach((key) => { | |
| if (key === '__proto__' || key === 'constructor' || key === 'prototype') return; | |
| this.req.headers[key] = entry[key]; | |
| }); | |
| } | |
| }); | |
| return; | |
| } | |
| this.req.headers = headers; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 147-147: Merging request-derived input into a target object via Object.assign / lodash merge / extend / deep-set can pollute Object.prototype when the payload contains "proto", "constructor", or "prototype" keys. Use a prototype-pollution-safe merge (e.g. lodash >=4.17.12 with key filtering), strip dangerous keys, or build the result on Object.create(null).
Context: Object.assign(this.req.headers, entry)
Note: [CWE-1321] Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution').
(prototype-pollution-recursive-merge-javascript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-js/src/bruno-request.js` around lines 140 - 153, Update
setHeaders in the array-form merge path to filter out prototype-pollution keys
(__proto__, constructor, and prototype) from each script-supplied entry before
merging into this.req.headers. Preserve merging of all other valid header keys
and the existing object-form replacement behavior.
Source: Linters/SAST tools
1 participant
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Bug Fixes