Skip to content

Commit

Permalink
fix(headers): always send lowercase headers and strip undefined (BREA…
Browse files Browse the repository at this point in the history
…KING in rare cases) (#608)

BREAKING: If you previously sent `My-Header: foo` and `my-header: bar`,
both would get sent. Now, only one will.
If you previously sent `My-Header: undefined`, it would send as such.
Now, the header will not be sent.
  • Loading branch information
stainless-bot authored Jan 4, 2024
1 parent 045ee74 commit 4ea159f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 20 deletions.
63 changes: 50 additions & 13 deletions src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,18 +301,7 @@ export abstract class APIClient {
headers[this.idempotencyHeader] = options.idempotencyKey;
}

const reqHeaders: Record<string, string> = {
...(contentLength && { 'Content-Length': contentLength }),
...this.defaultHeaders(options),
...headers,
};
// let builtin fetch set the Content-Type for multipart bodies
if (isMultipartBody(options.body) && shimsKind !== 'node') {
delete reqHeaders['Content-Type'];
}

// Strip any headers being explicitly omitted with null
Object.keys(reqHeaders).forEach((key) => reqHeaders[key] === null && delete reqHeaders[key]);
const reqHeaders = this.buildHeaders({ options, headers, contentLength });

const req: RequestInit = {
method,
Expand All @@ -324,9 +313,35 @@ export abstract class APIClient {
signal: options.signal ?? null,
};

return { req, url, timeout };
}

private buildHeaders({
options,
headers,
contentLength,
}: {
options: FinalRequestOptions;
headers: Record<string, string | null | undefined>;
contentLength: string | null | undefined;
}): Record<string, string> {
const reqHeaders: Record<string, string> = {};
if (contentLength) {
reqHeaders['content-length'] = contentLength;
}

const defaultHeaders = this.defaultHeaders(options);
applyHeadersMut(reqHeaders, defaultHeaders);
applyHeadersMut(reqHeaders, headers);

// let builtin fetch set the Content-Type for multipart bodies
if (isMultipartBody(options.body) && shimsKind !== 'node') {
delete reqHeaders['content-type'];
}

this.validateHeaders(reqHeaders, headers);

return { req, url, timeout };
return reqHeaders;
}

/**
Expand Down Expand Up @@ -1013,6 +1028,28 @@ export function hasOwn(obj: Object, key: string): boolean {
return Object.prototype.hasOwnProperty.call(obj, key);
}

/**
* Copies headers from "newHeaders" onto "targetHeaders",
* using lower-case for all properties,
* ignoring any keys with undefined values,
* and deleting any keys with null values.
*/
function applyHeadersMut(targetHeaders: Headers, newHeaders: Headers): void {
for (const k in newHeaders) {
if (!hasOwn(newHeaders, k)) continue;
const lowerKey = k.toLowerCase();
if (!lowerKey) continue;

const val = newHeaders[k];

if (val === null) {
delete targetHeaders[lowerKey];
} else if (val !== undefined) {
targetHeaders[lowerKey] = val;
}
}
}

export function debug(action: string, ...args: any[]) {
if (typeof process !== 'undefined' && process.env['DEBUG'] === 'true') {
console.log(`OpenAI:DEBUG:${action}`, ...args);
Expand Down
29 changes: 22 additions & 7 deletions tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,25 @@ describe('instantiate client', () => {

test('they are used in the request', () => {
const { req } = client.buildRequest({ path: '/foo', method: 'post' });
expect((req.headers as Headers)['X-My-Default-Header']).toEqual('2');
expect((req.headers as Headers)['x-my-default-header']).toEqual('2');
});

test('can be overriden with `undefined`', () => {
test('can ignore `undefined` and leave the default', () => {
const { req } = client.buildRequest({
path: '/foo',
method: 'post',
headers: { 'X-My-Default-Header': undefined },
});
expect((req.headers as Headers)['X-My-Default-Header']).toBeUndefined();
expect((req.headers as Headers)['x-my-default-header']).toEqual('2');
});

test('can be overriden with `null`', () => {
test('can be removed with `null`', () => {
const { req } = client.buildRequest({
path: '/foo',
method: 'post',
headers: { 'X-My-Default-Header': null },
});
expect((req.headers as Headers)['X-My-Default-Header']).toBeUndefined();
expect(req.headers as Headers).not.toHaveProperty('x-my-default-header');
});
});

Expand Down Expand Up @@ -179,12 +179,27 @@ describe('request building', () => {
describe('Content-Length', () => {
test('handles multi-byte characters', () => {
const { req } = client.buildRequest({ path: '/foo', method: 'post', body: { value: '—' } });
expect((req.headers as Record<string, string>)['Content-Length']).toEqual('20');
expect((req.headers as Record<string, string>)['content-length']).toEqual('20');
});

test('handles standard characters', () => {
const { req } = client.buildRequest({ path: '/foo', method: 'post', body: { value: 'hello' } });
expect((req.headers as Record<string, string>)['Content-Length']).toEqual('22');
expect((req.headers as Record<string, string>)['content-length']).toEqual('22');
});
});

describe('custom headers', () => {
test('handles undefined', () => {
const { req } = client.buildRequest({
path: '/foo',
method: 'post',
body: { value: 'hello' },
headers: { 'X-Foo': 'baz', 'x-foo': 'bar', 'x-Foo': undefined, 'x-baz': 'bam', 'X-Baz': null },
});
expect((req.headers as Record<string, string>)['x-foo']).toEqual('bar');
expect((req.headers as Record<string, string>)['x-Foo']).toEqual(undefined);
expect((req.headers as Record<string, string>)['X-Foo']).toEqual(undefined);
expect((req.headers as Record<string, string>)['x-baz']).toEqual(undefined);
});
});
});
Expand Down

0 comments on commit 4ea159f

Please sign in to comment.