-
Notifications
You must be signed in to change notification settings - Fork 225
fix(respect): record request bodies in the HAR postData entry #2992
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ariesclark
wants to merge
1
commit into
Redocly:main
Choose a base branch
from
ariesclark:fix/respect-har-post-data
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+104
−2
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@redocly/cli': patch | ||
| --- | ||
|
|
||
| Fixed `respect --har-output` recording an empty `postData` for every request. Request bodies are now written to the HAR, so a capture replayed through `drift` can have its request bodies validated instead of silently passing. |
60 changes: 60 additions & 0 deletions
60
packages/cli/src/__tests__/commands/respect/har-logs/helpers/build-post-data.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { buildPostData } from '../../../../../commands/respect/har-logs/helpers/build-post-data.js'; | ||
|
|
||
| describe('buildPostData', () => { | ||
| it('records a JSON body with the content type the request declared', () => { | ||
| const headers = { 'content-type': 'application/json' }; | ||
|
|
||
| expect(buildPostData('{"bio":"x"}', headers)).toEqual({ | ||
| mimeType: 'application/json', | ||
| text: '{"bio":"x"}', | ||
| }); | ||
| }); | ||
|
|
||
| it('matches the content-type header regardless of its casing', () => { | ||
| expect(buildPostData('{}', { 'Content-Type': 'application/json' }).mimeType).toBe( | ||
| 'application/json' | ||
| ); | ||
| }); | ||
|
|
||
| it('reads the content type from the flat array header form', () => { | ||
| const headers = ['accept', '*/*', 'content-type', 'application/json']; | ||
|
|
||
| expect(buildPostData('{}', headers).mimeType).toBe('application/json'); | ||
| }); | ||
|
|
||
| it('reads the content type from a Headers-like object', () => { | ||
| const headers = new Map([['content-type', 'application/json']]); | ||
|
|
||
| expect(buildPostData('{}', headers).mimeType).toBe('application/json'); | ||
| }); | ||
|
|
||
| it('falls back to application/octet-stream when no content type was declared', () => { | ||
| expect(buildPostData('raw', {}).mimeType).toBe('application/octet-stream'); | ||
| }); | ||
|
|
||
| it('returns an empty object for a request with no body, as before', () => { | ||
| expect(buildPostData(undefined, {})).toEqual({}); | ||
| }); | ||
|
|
||
| it('treats an empty string body as no body', () => { | ||
| expect(buildPostData('', {})).toEqual({}); | ||
| }); | ||
|
|
||
| it('serializes a URLSearchParams body', () => { | ||
| const body = new URLSearchParams({ a: '1', b: '2' }); | ||
|
|
||
| expect(buildPostData(body, {}).text).toBe('a=1&b=2'); | ||
| }); | ||
|
|
||
| it('does not attempt to serialize a stream body', () => { | ||
| expect(buildPostData(Buffer.from('binary'), {})).toEqual({}); | ||
| }); | ||
|
|
||
| it('omits a non-string body rather than recording "[object Object]"', () => { | ||
| expect(buildPostData({ bio: 'x' }, { 'content-type': 'application/json' })).toEqual({}); | ||
| }); | ||
|
|
||
| it('omits a FormData body, which cannot be read without consuming it', () => { | ||
| expect(buildPostData(new FormData(), { 'content-type': 'multipart/form-data' })).toEqual({}); | ||
| }); | ||
| }); |
35 changes: 35 additions & 0 deletions
35
packages/cli/src/commands/respect/har-logs/helpers/build-post-data.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { buildHeaders } from './build-headers.js'; | ||
|
|
||
| /** | ||
| * The HAR `postData` entry for a request body. | ||
| * | ||
| * Without this the capture records the request line and headers but not what | ||
| * was sent, so anything reading the HAR back — `drift`'s request-body | ||
| * validation, for one — silently has nothing to check. | ||
| */ | ||
| export function buildPostData( | ||
| body: unknown, | ||
| headers: any = {} | ||
| ): { mimeType?: string; text?: string } { | ||
| const text = serializeBody(body); | ||
| if (text === undefined || text === '') return {}; | ||
|
|
||
| // Via `buildHeaders`, so every shape a request can carry its headers in is | ||
| // handled the same way here as it is for the entry's `headers` list. | ||
| const contentType = buildHeaders(headers).find( | ||
| ({ name }) => String(name).toLowerCase() === 'content-type' | ||
| )?.value; | ||
|
|
||
| return { | ||
| mimeType: typeof contentType === 'string' ? contentType : 'application/octet-stream', | ||
| text, | ||
| }; | ||
| } | ||
|
|
||
| /** Only bodies that are already text; a stream or binary body is left out. */ | ||
| function serializeBody(body: unknown): string | undefined { | ||
| if (typeof body === 'string') return body; | ||
| if (body instanceof URLSearchParams) return body.toString(); | ||
|
|
||
| return undefined; | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { URL } from 'url'; | |||||||||||
|
|
||||||||||||
| import { addHeaders } from './helpers/add-headers.js'; | ||||||||||||
| import { buildHeaders } from './helpers/build-headers.js'; | ||||||||||||
| import { buildPostData } from './helpers/build-post-data.js'; | ||||||||||||
| import { buildRequestCookies } from './helpers/build-request-cookies.js'; | ||||||||||||
| import { buildResponseCookies } from './helpers/build-response-cookies.js'; | ||||||||||||
| import { getDuration } from './helpers/get-duration.js'; | ||||||||||||
|
|
@@ -43,6 +44,7 @@ export const withHar: WithHar = function <T extends typeof fetch>( | |||||||||||
| const startTime = process.hrtime(); | ||||||||||||
|
|
||||||||||||
| const url = new URL(typeof input === 'string' ? input : input.url); | ||||||||||||
| const postData = buildPostData(options.body, options.headers || {}); | ||||||||||||
|
|
||||||||||||
| const entry = { | ||||||||||||
| _compressed: false, | ||||||||||||
|
|
@@ -82,8 +84,8 @@ export const withHar: WithHar = function <T extends typeof fetch>( | |||||||||||
| value, | ||||||||||||
| })), | ||||||||||||
| headersSize: -1, | ||||||||||||
| bodySize: -1, | ||||||||||||
| postData: {}, | ||||||||||||
| bodySize: postData.text === undefined ? -1 : Buffer.byteLength(postData.text), | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| postData, | ||||||||||||
| httpVersion: 'HTTP/1.1', | ||||||||||||
| }, | ||||||||||||
| response: {}, | ||||||||||||
|
|
||||||||||||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetch sets this content type itself when a URLSearchParams body is sent without an explicit header: