Skip to content

Commit

Permalink
fix: parse JSON body when Content-Type has charset
Browse files Browse the repository at this point in the history
Or other parameters.

In accordance with RFC 1341[^1], Content-Type contains first and
foremost the media type. It can be followed by key-value pairs of
parameters.

This change supports extracting the media type and effectively
discarding the other parameters as these are currently irrelevant to
body parsing. The result media type is then matched against previously
used JSON type tests.

Closes apollographql#229
Closes apollographql#341

[^1]:https://www.rfc-editor.org/rfc/rfc1341
  • Loading branch information
slagiewka committed Dec 22, 2024
1 parent 9ac8acf commit c6a9eaa
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/unlucky-geese-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/datasource-rest': patch
---

Parse JSON body when Content-Type has charset
6 changes: 3 additions & 3 deletions src/RESTDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,14 @@ export abstract class RESTDataSource<CO extends CacheOptions = CacheOptions> {
protected parseBody(response: FetcherResponse): Promise<object | string> {
const contentType = response.headers.get('Content-Type');
const contentLength = response.headers.get('Content-Length');
const mediaType = contentType?.split(';')?.[0];
if (
// As one might expect, a "204 No Content" is empty! This means there
// isn't enough to `JSON.parse`, and trying will result in an error.
response.status !== 204 &&
contentLength !== '0' &&
contentType &&
(contentType.startsWith('application/json') ||
contentType.endsWith('+json'))
(mediaType?.startsWith('application/json') ||
mediaType?.endsWith('+json'))
) {
return response.json();
} else {
Expand Down
45 changes: 45 additions & 0 deletions src/__tests__/RESTDataSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,51 @@ describe('RESTDataSource', () => {
expect(data).toEqual({ foo: 'bar' });
});

it('returns data as parsed JSON when Content-Type ends in +json and has charset provided', async () => {
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

getFoo() {
return this.get('foo');
}
})();

nock(apiUrl)
.get('/foo')
.reply(
200,
{ foo: 'bar' },
{ 'content-type': 'application/vnd.api+json; charset=utf-8' },
);

const data = await dataSource.getFoo();

expect(data).toEqual({ foo: 'bar' });
});

it('returns data as parsed JSON when Content-Type ends in +json and has many parameters provided', async () => {
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';

getFoo() {
return this.get('foo');
}
})();

nock(apiUrl).get('/foo').reply(
200,
{ foo: 'bar' },
{
'content-type':
'application/vnd.api+json; charset=utf-8; boundary=ExampleBoundaryString',
},
);

const data = await dataSource.getFoo();

expect(data).toEqual({ foo: 'bar' });
});

it('returns data as a string when Content-Type is text/plain', async () => {
const dataSource = new (class extends RESTDataSource {
override baseURL = 'https://api.example.com';
Expand Down

0 comments on commit c6a9eaa

Please sign in to comment.