Skip to content

Commit

Permalink
Clone URLSearchParams to avoid mutation (#547)
Browse files Browse the repository at this point in the history
* And make sure Request/Response set Content-Type per Fetch Spec
* And make sure users can read the body as string via text()
  • Loading branch information
jimmywarting authored and bitinn committed Nov 13, 2018
1 parent 5367fe6 commit 2d0fc68
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 9 deletions.
10 changes: 6 additions & 4 deletions src/body.js
Expand Up @@ -37,6 +37,7 @@ export default function Body(body, {
} else if (typeof body === 'string') {
// body is string
} else if (isURLSearchParams(body)) {
body = Buffer.from(body.toString());
// body is a URLSearchParams
} else if (body instanceof Blob) {
// body is blob
Expand Down Expand Up @@ -415,9 +416,7 @@ export function clone(instance) {
*
* @param Mixed instance Response or Request instance
*/
export function extractContentType(instance) {
const {body} = instance;

export function extractContentType(body) {
// istanbul ignore if: Currently, because of a guard in Request, body
// can never be null. Included here for completeness.
if (body === null) {
Expand All @@ -444,10 +443,13 @@ export function extractContentType(instance) {
} else if (typeof body.getBoundary === 'function') {
// detect form data input from form-data module
return `multipart/form-data;boundary=${body.getBoundary()}`;
} else {
} else if (body instanceof Stream) {
// body is stream
// can't really do much about this
return null;
} else {
// Body constructor defaults other things to string
return 'text/plain;charset=UTF-8';
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/request.js
Expand Up @@ -90,9 +90,9 @@ export default class Request {

const headers = new Headers(init.headers || input.headers || {});

if (init.body != null) {
const contentType = extractContentType(this);
if (contentType !== null && !headers.has('Content-Type')) {
if (inputBody != null && !headers.has('Content-Type')) {
const contentType = extractContentType(inputBody);
if (contentType) {
headers.append('Content-Type', contentType);
}
}
Expand Down
12 changes: 10 additions & 2 deletions src/response.js
Expand Up @@ -8,7 +8,7 @@
import http from 'http';

import Headers from './headers.js';
import Body, { clone } from './body';
import Body, { clone, extractContentType } from './body';

const INTERNALS = Symbol('Response internals');

Expand All @@ -27,12 +27,20 @@ export default class Response {
Body.call(this, body, opts);

const status = opts.status || 200;
const headers = new Headers(opts.headers)

if (body != null && !headers.has('Content-Type')) {
const contentType = extractContentType(body);
if (contentType) {
headers.append('Content-Type', contentType);
}
}

this[INTERNALS] = {
url: opts.url,
status,
statusText: opts.statusText || STATUS_CODES[status],
headers: new Headers(opts.headers)
headers
};
}

Expand Down
32 changes: 32 additions & 0 deletions test/test.js
Expand Up @@ -1365,6 +1365,38 @@ describe('node-fetch', () => {
});

const itUSP = typeof URLSearchParams === 'function' ? it : it.skip;

itUSP('constructing a Response with URLSearchParams as body should have a Content-Type', function() {
const params = new URLSearchParams();
const res = new Response(params);
res.headers.get('Content-Type');
expect(res.headers.get('Content-Type')).to.equal('application/x-www-form-urlencoded;charset=UTF-8');
});

itUSP('constructing a Request with URLSearchParams as body should have a Content-Type', function() {
const params = new URLSearchParams();
const req = new Request(base, { method: 'POST', body: params });
expect(req.headers.get('Content-Type')).to.equal('application/x-www-form-urlencoded;charset=UTF-8');
});

itUSP('Reading a body with URLSearchParams should echo back the result', function() {
const params = new URLSearchParams();
params.append('a','1');
return new Response(params).text().then(text => {
expect(text).to.equal('a=1');
});
});

// Body should been cloned...
itUSP('constructing a Request/Response with URLSearchParams and mutating it should not affected body', function() {
const params = new URLSearchParams();
const req = new Request(`${base}inspect`, { method: 'POST', body: params })
params.append('a','1')
return req.text().then(text => {
expect(text).to.equal('');
});
});

itUSP('should allow POST request with URLSearchParams as body', function() {
const params = new URLSearchParams();
params.append('a','1');
Expand Down

0 comments on commit 2d0fc68

Please sign in to comment.