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: Duplicate Content-Type header when Content-Type header is capitalized. #154

Merged
Show file tree
Hide file tree
Changes from 5 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
5 changes: 5 additions & 0 deletions .changeset/rude-planets-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/datasource-rest': patch
---

Addresses duplicate content-type header bug due to upper-cased headers being forwarded. This change instead maps all headers to lowercased headers.
14 changes: 10 additions & 4 deletions src/RESTDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,14 +454,22 @@ export abstract class RESTDataSource {
path: string,
incomingRequest: DataSourceRequest = {},
): Promise<DataSourceFetchResult<TResult>> {
const downcasedHeaders: Record<string, string> = {};
if (incomingRequest.headers) {
// map incoming headers to lower-case headers
Object.entries(incomingRequest.headers).forEach(([key, value]) => {
downcasedHeaders[key.toLowerCase()] = value;
});
}

const augmentedRequest: AugmentedRequest = {
...incomingRequest,
// guarantee params and headers objects before calling `willSendRequest` for convenience
params:
incomingRequest.params instanceof URLSearchParams
? incomingRequest.params
: this.urlSearchParamsFromRecord(incomingRequest.params),
headers: incomingRequest.headers ?? Object.create(null),
headers: downcasedHeaders,
};
// Default to GET in the case that `fetch` is called directly with no method
// provided. Our other request methods all provide one.
Expand All @@ -481,9 +489,7 @@ export abstract class RESTDataSource {
if (this.shouldJSONSerializeBody(augmentedRequest.body)) {
augmentedRequest.body = JSON.stringify(augmentedRequest.body);
// If Content-Type header has not been previously set, set to application/json
if (!augmentedRequest.headers) {
augmentedRequest.headers = { 'content-type': 'application/json' };
} else if (!augmentedRequest.headers['content-type']) {
if (!augmentedRequest.headers['content-type']) {
augmentedRequest.headers['content-type'] = 'application/json';
}
}
Expand Down
128 changes: 128 additions & 0 deletions src/__tests__/RESTDataSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,134 @@ describe('RESTDataSource', () => {
await dataSource.getFoo('1');
});

it('contains only one Content-Type header when Content-Type already exists is application/json', async () => {
const requestOptions = {
headers: {
'Content-Type': 'application/json',
},
body: { foo: 'bar' },
};
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

postFoo() {
return this.post('foo', requestOptions);
}
})();

const spyOnHttpFetch = jest.spyOn(dataSource['httpCache'], 'fetch');

nock(apiUrl)
.post('/foo')
.reply(200, { foo: 'bar' }, { 'Content-Type': 'application/json' });

const data = await dataSource.postFoo();
expect(spyOnHttpFetch.mock.calls[0][1]).toEqual({
headers: { 'content-type': 'application/json' },
body: '{"foo":"bar"}',
method: 'POST',
params: new URLSearchParams(),
});
expect(data).toEqual({ foo: 'bar' });
});

it('converts uppercase-containing headers to lowercase', async () => {
const requestOptions = {
headers: {
'Content-Type': 'application/json',
'Test-Header': 'foobar',
'ANOTHER-TEST-HEADER': 'test2',
},
body: { foo: 'bar' },
};
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

postFoo() {
return this.post('foo', requestOptions);
}
})();

const spyOnHttpFetch = jest.spyOn(dataSource['httpCache'], 'fetch');

nock(apiUrl)
.post('/foo')
.reply(200, { foo: 'bar' }, { 'Content-Type': 'application/json' });

const data = await dataSource.postFoo();
expect(spyOnHttpFetch.mock.calls[0][1]).toEqual({
headers: {
'content-type': 'application/json',
'test-header': 'foobar',
'another-test-header': 'test2',
},
body: '{"foo":"bar"}',
method: 'POST',
params: new URLSearchParams(),
});
expect(data).toEqual({ foo: 'bar' });
});

it('adds an `application/json` content-type header when none is present', async () => {
const requestOptions = {
body: { foo: 'bar' },
};
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

postFoo() {
return this.post('foo', requestOptions);
}
})();

const spyOnHttpFetch = jest.spyOn(dataSource['httpCache'], 'fetch');

nock(apiUrl)
.post('/foo')
.reply(200, { foo: 'bar' }, { 'Content-Type': 'application/json' });

const data = await dataSource.postFoo();
expect(spyOnHttpFetch.mock.calls[0][1]).toEqual({
headers: {
'content-type': 'application/json',
},
body: '{"foo":"bar"}',
method: 'POST',
params: new URLSearchParams(),
});
expect(data).toEqual({ foo: 'bar' });
});

it('adds an `application/json` content header when no headers are passed in', async () => {
const requestOptions = {
body: {foo: 'bar' },
};
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

postFoo() {
return this.post('foo', requestOptions);
}
})();

const spyOnHttpFetch = jest.spyOn(dataSource['httpCache'], 'fetch');

nock(apiUrl)
.post('/foo')
.reply(200, { foo: 'bar' }, { 'Content-Type': 'application/json' });

const data = await dataSource.postFoo();
expect(spyOnHttpFetch.mock.calls[0][1]).toEqual({
headers: {
'content-type': 'application/json',
},
body: '{"foo":"bar"}',
method: 'POST',
params: new URLSearchParams(),
});
expect(data).toEqual({ foo: 'bar' });
});

it('serializes a request body that is an object as JSON', async () => {
const expectedFoo = { foo: 'bar' };
const dataSource = new (class extends RESTDataSource {
Expand Down