Skip to content
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

fix: various quirks with how we handle accept headers #699

Merged
merged 3 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/api/src/core/prepareParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
104 changes: 104 additions & 0 deletions packages/api/test/__fixtures__/definitions/basiq.json
kanadgupta marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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"]
}
}
35 changes: 35 additions & 0 deletions packages/api/test/core/prepareParams.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
11 changes: 6 additions & 5 deletions packages/httpsnippet-client-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -224,10 +226,9 @@ const client: Client<APIOptions> = {
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)) {
kanadgupta marked this conversation as resolved.
Show resolved Hide resolved
delete headers[header];
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));