From e60c3717a9de3558cd44127ed14bd8940370ee66 Mon Sep 17 00:00:00 2001 From: Jon Ursenbach Date: Fri, 18 Aug 2023 12:55:49 -0700 Subject: [PATCH] fix: various quirks with how we handle `accept` headers (#699) * fix(api): quirk where headers could be duped in body payloads * fix(api): stop sending special headers in body payloads * fix(snippet): stop putting `accept` headers in snippets if we can --- packages/api/src/core/prepareParams.ts | 11 +- .../test/__fixtures__/definitions/basiq.json | 104 ++++++++++++++++++ packages/api/test/core/prepareParams.test.ts | 35 ++++++ packages/httpsnippet-client-api/src/index.ts | 11 +- .../full-many-query-params/index.ts | 2 +- .../full-many-query-params/output.js | 3 +- .../test/__datasets__/full/index.ts | 2 +- .../test/__datasets__/full/output.js | 3 +- 8 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 packages/api/test/__fixtures__/definitions/basiq.json diff --git a/packages/api/src/core/prepareParams.ts b/packages/api/src/core/prepareParams.ts index e2a7c389..13f54fad 100644 --- a/packages/api/src/core/prepareParams.ts +++ b/packages/api/src/core/prepareParams.ts @@ -332,6 +332,7 @@ export default async function prepareParams(operation: Operation, body?: unknown if (typeof metadata === 'object' && !isEmpty(metadata)) { if (paramName in metadata) { value = metadata[paramName]; + metadataHeaderParam = paramName; } else if (param.in === 'header') { // Headers are sent case-insensitive so we need to make sure that we're properly // matching them when detecting what our incoming payload looks like. @@ -379,9 +380,7 @@ export default async function prepareParams(operation: Operation, body?: unknown // If there's any leftover metadata that hasn't been moved into form data for this request we // need to move it or else it'll get tossed. if (!isEmpty(metadata)) { - if (operation.isFormUrlEncoded()) { - params.formData = merge(params.formData, metadata); - } else if (typeof metadata === 'object') { + if (typeof metadata === 'object') { // If the user supplied an `accept` or `authorization` header themselves we should allow it // through. Normally these headers are automatically handled by `@readme/oas-to-har` but in // the event that maybe the user wants to return XML for an API that normally returns JSON @@ -391,8 +390,14 @@ export default async function prepareParams(operation: Operation, body?: unknown const headerParam = Object.keys(metadata).find(m => m.toLowerCase() === headerName); if (headerParam) { params.header[headerName] = metadata[headerParam] as string; + // eslint-disable-next-line no-param-reassign + delete metadata[headerParam]; } }); + } + + if (operation.isFormUrlEncoded()) { + params.formData = merge(params.formData, metadata); } else { // Any other remaining unused metadata will be unused because we don't know where to place // it in the request. diff --git a/packages/api/test/__fixtures__/definitions/basiq.json b/packages/api/test/__fixtures__/definitions/basiq.json new file mode 100644 index 00000000..b294007d --- /dev/null +++ b/packages/api/test/__fixtures__/definitions/basiq.json @@ -0,0 +1,104 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "Core", + "description": "All included utility endpoints for Basiq partners", + "version": "3.0.0" + }, + "servers": [ + { + "url": "https://au-api.basiq.io/" + } + ], + "paths": { + "/token": { + "post": { + "tags": ["Authentication"], + "summary": "Generate an auth token", + "description": "Use this endpoint to retrieve a token that will be passed as authorization header for Basiq API", + "operationId": "postToken", + "parameters": [ + { + "name": "basiq-version", + "in": "header", + "required": true, + "schema": { + "type": "string", + "example": "3.0" + } + } + ], + "requestBody": { + "content": { + "application/x-www-form-urlencoded": { + "schema": { + "properties": { + "scope": { + "type": "string" + }, + "userId": { + "type": "string" + } + } + }, + "examples": { + "client_access": { + "summary": "For all client side requests", + "value": { + "scope": "CLIENT_ACCESS", + "userId": "6dd30ce4-d4ba-11ec-9d64-0242ac120002" + } + }, + "server_access": { + "summary": "For all server side requests", + "value": { + "scope": "SERVER_ACCESS" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "OK" + } + }, + "security": [ + { + "api_key": [] + } + ] + } + } + }, + "components": { + "securitySchemes": { + "api_key": { + "type": "apiKey", + "name": "Authorization", + "in": "header", + "x-default": "Basic NjMxMjNmMWMtZjYxMy00ZjMyLWFiYzUtYzBhZDdhYTY2YmU1OjQ3NWYwMzhkLTBlZmItNGM1ZS1iMzQ0LTAzMzYxOTkyYTRlMw==" + }, + "services_token": { + "type": "http", + "scheme": "bearer", + "bearerFormat": "JWT" + } + } + }, + "security": [ + { + "api_key": [] + }, + { + "services_token": [] + } + ], + "x-readme": { + "explorer-enabled": true, + "proxy-enabled": true, + "samples-enabled": true, + "samples-languages": ["curl", "node", "ruby", "javascript", "python"] + } +} diff --git a/packages/api/test/core/prepareParams.test.ts b/packages/api/test/core/prepareParams.test.ts index ddbf2f6a..6af34051 100644 --- a/packages/api/test/core/prepareParams.test.ts +++ b/packages/api/test/core/prepareParams.test.ts @@ -379,6 +379,41 @@ describe('#prepareParams', () => { }); }); + describe('quirks', () => { + it('should not send special headers in body payloads', async () => { + const basiq = await import('../__fixtures__/definitions/basiq.json').then(Oas.init); + await basiq.dereference(); + + const operation = basiq.operation('/token', 'post'); + + await expect( + prepareParams(operation, { scope: 'SERVER_ACCESS' }, { accept: 'application/json', 'BASIQ-version': '3.0' }), + ).resolves.toStrictEqual({ + formData: { + scope: 'SERVER_ACCESS', + }, + header: { + accept: 'application/json', + 'basiq-version': '3.0', + }, + }); + }); + + it('should not duplicate a supplied header parameter if that header casing matches the spec', async () => { + const basiq = await import('../__fixtures__/definitions/basiq.json').then(Oas.init); + await basiq.dereference(); + + const operation = basiq.operation('/token', 'post'); + + await expect( + prepareParams(operation, { scope: 'scope', userId: 'userid' }, { 'basiq-version': '3.0' }), + ).resolves.toStrictEqual({ + formData: { scope: 'scope', userId: 'userid' }, + header: { 'basiq-version': '3.0' }, + }); + }); + }); + describe('`accept` header overrides', () => { it('should support supplying an `accept` header parameter', async () => { const operation = parameterStyle.operation('/anything/headers', 'get'); diff --git a/packages/httpsnippet-client-api/src/index.ts b/packages/httpsnippet-client-api/src/index.ts index 71857d3b..b5ea2fd3 100644 --- a/packages/httpsnippet-client-api/src/index.ts +++ b/packages/httpsnippet-client-api/src/index.ts @@ -5,9 +5,11 @@ import type { HttpMethods, OASDocument } from 'oas/dist/rmoas.types'; import { CodeBuilder } from '@readme/httpsnippet/dist/helpers/code-builder'; import contentType from 'content-type'; -import Oas from 'oas'; +import Oas, { utils } from 'oas'; import stringifyObject from 'stringify-object'; +const { matchesMimeType } = utils; + // This should really be an exported type in `oas`. type SecurityType = 'Basic' | 'Bearer' | 'Query' | 'Header' | 'Cookie' | 'OAuth2' | 'http' | 'apiKey'; @@ -224,10 +226,9 @@ const client: Client = { return; } } else if (headerLower === 'accept') { - // If the `Accept` header here is not the default or first `Accept` header for the - // operations' request body then we should add it otherwise we can let the SDK handle it - // itself. - if (headers[header] === operation.getContentType()) { + // If the `Accept` header here is JSON-like header then we can remove it from the code + // snippet because `api` natively supports and prioritizes JSON over any other mime type. + if (matchesMimeType.json(headers[header] as string)) { delete headers[header]; return; } diff --git a/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/index.ts b/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/index.ts index bb1baf4c..2a235501 100644 --- a/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/index.ts +++ b/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/index.ts @@ -81,7 +81,7 @@ const mock: SnippetMock = { cookie: 'bar-cookie=baz; foo-cookie=bar', }, functionMatcher: (url, opts) => { - return opts.body === 'foo=bar&foo2=bar2&foo3=bar3&foo4=bar4&accept=application%2Fjson'; + return opts.body === 'foo=bar&foo2=bar2&foo3=bar3&foo4=bar4'; }, }, res: { diff --git a/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/output.js b/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/output.js index 8b12e4e7..da01c5cb 100644 --- a/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/output.js +++ b/packages/httpsnippet-client-api/test/__datasets__/full-many-query-params/output.js @@ -10,8 +10,7 @@ sdk.postAnything({ baz: 'abc', key: 'value', 'bar-cookie': 'baz', - 'foo-cookie': 'bar', - accept: 'application/json' + 'foo-cookie': 'bar' }) .then(({ data }) => console.log(data)) .catch(err => console.error(err)); diff --git a/packages/httpsnippet-client-api/test/__datasets__/full/index.ts b/packages/httpsnippet-client-api/test/__datasets__/full/index.ts index bc0ca2c6..dfffc830 100644 --- a/packages/httpsnippet-client-api/test/__datasets__/full/index.ts +++ b/packages/httpsnippet-client-api/test/__datasets__/full/index.ts @@ -69,7 +69,7 @@ const mock: SnippetMock = { cookie: 'bar-cookie=baz; foo-cookie=bar', }, functionMatcher: (url, opts) => { - return opts.body === 'foo=bar&accept=application%2Fjson'; + return opts.body === 'foo=bar'; }, }, res: { diff --git a/packages/httpsnippet-client-api/test/__datasets__/full/output.js b/packages/httpsnippet-client-api/test/__datasets__/full/output.js index 5347cb9f..5b7a6304 100644 --- a/packages/httpsnippet-client-api/test/__datasets__/full/output.js +++ b/packages/httpsnippet-client-api/test/__datasets__/full/output.js @@ -5,8 +5,7 @@ sdk.postAnything({foo: 'bar'}, { baz: 'abc', key: 'value', 'bar-cookie': 'baz', - 'foo-cookie': 'bar', - accept: 'application/json' + 'foo-cookie': 'bar' }) .then(({ data }) => console.log(data)) .catch(err => console.error(err));