Change stringifying options behavior (#297)

index.js

@@ -16,6 +16,7 @@ const unzipResponse = require('unzip-response');
 const createErrorClass = require('create-error-class');
 const isRetryAllowed = require('is-retry-allowed');
 const Buffer = require('safe-buffer').Buffer;
+const isPlainObj = require('is-plain-obj');
 const pkg = require('./package');
 
 function requestAsEventEmitter(opts) {
@@ -243,31 +244,37 @@ function normalizeArguments(url, opts) {
 		opts.headers.accept = 'application/json';
 	}
 
-	let body = opts.body;
+	const body = opts.body;
-
+	if (body !== null && body !== undefined) {
-	if (body) {
+		const headers = opts.headers;
-		if (typeof body !== 'string' && !(body !== null && typeof body === 'object')) {
+		if (!isStream(body) && typeof body !== 'string' && !Buffer.isBuffer(body) && !(opts.form || opts.json)) {
-			throw new Error('options.body must be a ReadableStream, string, Buffer or plain Object');
+			throw new TypeError('options.body must be a ReadableStream, string, Buffer or plain Object');
 		}
 
-		opts.method = opts.method || 'POST';
+		if ((opts.form || opts.json) && !isPlainObj(body)) {
+			throw new TypeError('options.body must be a plain Object when options.form or options.json is used');
+		}
 
 		if (isStream(body) && typeof body.getBoundary === 'function') {
 			// Special case for https://github.com/form-data/form-data
-			opts.headers['content-type'] = opts.headers['content-type'] || `multipart/form-data; boundary=${body.getBoundary()}`;
+			headers['content-type'] = headers['content-type'] || `multipart/form-data; boundary=${body.getBoundary()}`;
-		} else if (body !== null && typeof body === 'object' && !Buffer.isBuffer(body) && !isStream(body)) {
+		} else if (opts.form && isPlainObj(body)) {
-			opts.headers['content-type'] = opts.headers['content-type'] || 'application/x-www-form-urlencoded';
+			headers['content-type'] = headers['content-type'] || 'application/x-www-form-urlencoded';
-			body = querystring.stringify(body);
+			opts.body = querystring.stringify(body);
-			opts.body = body;
+		} else if (opts.json && isPlainObj(body)) {
+			headers['content-type'] = headers['content-type'] || 'application/json';
+			opts.body = JSON.stringify(body);
 		}
 
-		if (opts.headers['content-length'] === undefined && opts.headers['transfer-encoding'] === undefined && !isStream(body)) {
+		if (headers['content-length'] === undefined && headers['transfer-encoding'] === undefined && !isStream(body)) {
-			const length = typeof body === 'string' ? Buffer.byteLength(body) : body.length;
+			const length = typeof opts.body === 'string' ? Buffer.byteLength(opts.body) : opts.body.length;
-			opts.headers['content-length'] = length;
+			headers['content-length'] = length;
-		}
 		}
 
+		opts.method = (opts.method || 'POST').toUpperCase();
+	} else {
 		opts.method = (opts.method || 'GET').toUpperCase();
+	}
 
 	if (opts.hostname === 'unix') {
 		const matches = /(.+):(.+)/.exec(opts.path);

package.json

@@ -45,6 +45,7 @@
     "create-error-class": "^3.0.0",
     "duplexer3": "^0.1.4",
     "get-stream": "^3.0.0",
+    "is-plain-obj": "^1.1.0",
     "is-redirect": "^1.0.0",
     "is-retry-allowed": "^1.0.0",
     "is-stream": "^1.0.0",

readme.md

@@ -74,7 +74,7 @@ Any of the [`http.request`](http://nodejs.org/api/http.html#http_http_request_op
 
 ###### body
 
-Type: `string`, `buffer`, `readableStream`, `object`
+Type: `string`, `buffer`, `readableStream`
 
 *This is mutually exclusive with stream mode.*
 
@@ -84,8 +84,6 @@ If present in `options` and `options.method` is not set, `options.method` will b
 
 If `content-length` or `transfer-encoding` is not set in `options.headers` and `body` is a string or buffer, `content-length` will be set to the body length.
 
-If `body` is a plain object, it will be stringified with [`querystring.stringify`](https://nodejs.org/api/querystring.html#querystring_querystring_stringify_obj_sep_eq_options) and sent as `application/x-www-form-urlencoded`.
-
 ###### encoding
 
 Type: `string`, `null`<br>
@@ -93,6 +91,17 @@ Default: `'utf8'`
 
 Encoding to be used on `setEncoding` of the response data. If `null`, the body is returned as a Buffer.
 
+###### form
+
+Type: `boolean`<br>
+Default: `false`
+
+*This is mutually exclusive with stream mode.*
+
+If set to `true` and `Content-Type` header is not set, it will be set to `application/x-www-form-urlencoded`.
+
+`body` must be a plain object and will be stringified.
+
 ###### json
 
 Type: `boolean`<br>
@@ -100,7 +109,11 @@ Default: `false`
 
 *This is mutually exclusive with stream mode.*
 
-Parse response body with `JSON.parse` and set `accept` header to `application/json`.
+If set to `true` and `Content-Type` header is not set, it will be set to `application/json`.
+
+Parse response body with `JSON.parse` and set `accept` header to `application/json`. If used in conjunction with the `form` option, the `body` will the stringified as querystring and the response parsed as JSON.
+
+`body` must be a plain object and will be stringified.
 
 ###### query
 

test/arguments.js

@@ -61,6 +61,10 @@ test('should throw with auth in url', async t => {
 	}
 });
 
+test('should throw when body is set to object', async t => {
+	await t.throws(got(`${s.url}/`, {body: {}}), TypeError);
+});
+
 test.after('cleanup', async () => {
 	await s.close();
 });

test/headers.js

@@ -56,36 +56,36 @@ test('transform names to lowercase', async t => {
 });
 
 test('zero content-length', async t => {
-	const headers = (await got(s.url, {
+	const body = (await got(s.url, {
 		headers: {
 			'content-length': 0
 		},
-		body: 'sup',
+		body: 'sup'
-		json: true
 	})).body;
+	const headers = JSON.parse(body);
 	t.is(headers['content-length'], '0');
 });
 
 test('form-data manual content-type', async t => {
 	const form = new FormData();
 	form.append('a', 'b');
-	const headers = (await got(s.url, {
+	const body = (await got(s.url, {
 		headers: {
 			'content-type': 'custom'
 		},
-		body: form,
+		body: form
-		json: true
 	})).body;
+	const headers = JSON.parse(body);
 	t.is(headers['content-type'], 'custom');
 });
 
 test('form-data automatic content-type', async t => {
 	const form = new FormData();
 	form.append('a', 'b');
-	const headers = (await got(s.url, {
+	const body = (await got(s.url, {
-		body: form,
+		body: form
-		json: true
 	})).body;
+	const headers = JSON.parse(body);
 	t.is(headers['content-type'], `multipart/form-data; boundary=${form.getBoundary()}`);
 });
 

test/json.js → test/json-parse.js

@@ -30,6 +30,10 @@ test.before('setup', async () => {
 		res.end('Internal error');
 	});
 
+	s.on('/headers', (req, res) => {
+		res.end(JSON.stringify(req.headers));
+	});
+
 	await s.listen(s.port);
 });
 
@@ -79,6 +83,12 @@ test('should have statusCode in err', async t => {
 	t.is(err.statusCode, 500);
 });
 
+test('should set correct headers', async t => {
+	const {body: headers} = await got(`${s.url}/headers`, {json: true, body: {}});
+	t.is(headers['content-type'], 'application/json');
+	t.is(headers.accept, 'application/json');
+});
+
 test.after('cleanup', async () => {
 	await s.close();
 });

test/post.js

@@ -30,23 +30,6 @@ test('GET can have body', async t => {
 	t.is(headers.method, 'GET');
 });
 
-test('sends null-prototype objects', async t => {
-	const {body} = await got(s.url, {body: Object.create(null)});
-	t.is(body, '');
-});
-
-test('sends plain objects', async t => {
-	const {body} = await got(s.url, {body: {}});
-	t.is(body, '');
-});
-
-test('sends non-plain objects', async t => {
-	class Obj {}
-
-	const {body} = await got(s.url, {body: new Obj()});
-	t.is(body, '');
-});
-
 test('sends strings', async t => {
 	const {body} = await got(s.url, {body: 'wow'});
 	t.is(body, 'wow');
@@ -62,76 +45,71 @@ test('sends Streams', async t => {
 	t.is(body, 'wow');
 });
 
+test('sends plain objects as forms', async t => {
+	const {body} = await got(s.url, {
+		body: {such: 'wow'},
+		form: true
+	});
+	t.is(body, 'such=wow');
+});
+
+test('sends plain objects as JSON', async t => {
+	const {body} = await got(s.url, {
+		body: {such: 'wow'},
+		json: true
+	});
+	t.deepEqual(body, {such: 'wow'});
+});
+
 test('works with empty post response', async t => {
 	const {body} = await got(`${s.url}/empty`, {body: 'wow'});
 	t.is(body, '');
 });
 
 test('content-length header with string body', async t => {
-	const {body} = await got(`${s.url}/headers`, {
+	const {body} = await got(`${s.url}/headers`, {body: 'wow'});
-		body: 'wow',
+	const headers = JSON.parse(body);
-		json: true
+	t.is(headers['content-length'], '3');
-	});
-	t.is(body['content-length'], '3');
 });
 
 test('content-length header with Buffer body', async t => {
-	const {body} = await got(`${s.url}/headers`, {
+	const {body} = await got(`${s.url}/headers`, {body: Buffer.from('wow')});
-		body: Buffer.from('wow'),
+	const headers = JSON.parse(body);
-		json: true
+	t.is(headers['content-length'], '3');
-	});
-	t.is(body['content-length'], '3');
 });
 
 test('content-length header with Stream body', async t => {
-	const {body} = await got(`${s.url}/headers`, {
+	const {body} = await got(`${s.url}/headers`, {body: intoStream(['wow'])});
-		body: intoStream(['wow']),
+	const headers = JSON.parse(body);
-		json: true
+	t.is(headers['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
-	});
+	t.is(headers['content-length'], undefined);
-	t.is(body['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
-	t.is(body['content-length'], undefined);
 });
 
 test('content-length header is not overriden', async t => {
 	const {body} = await got(`${s.url}/headers`, {
 		body: 'wow',
-		json: true,
 		headers: {
 			'content-length': '10'
 		}
 	});
-	t.is(body['content-length'], '10');
+	const headers = JSON.parse(body);
+	t.is(headers['content-length'], '10');
 });
 
 test('content-length header disabled for chunked transfer-encoding', async t => {
 	const {body} = await got(`${s.url}/headers`, {
 		body: '3\r\nwow\r\n0\r\n',
-		json: true,
 		headers: {
 			'transfer-encoding': 'chunked'
 		}
 	});
-	t.is(body['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
+	const headers = JSON.parse(body);
-	t.is(body['content-length'], undefined);
+	t.is(headers['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
-});
+	t.is(headers['content-length'], undefined);
-
-test('object in options.body treated as querystring', async t => {
-	class Obj {
-		constructor() {
-			this.such = 'wow';
-		}
-
-		get ouch() {
-			return 'yay';
-		}
-	}
-
-	const {body} = await got(s.url, {body: new Obj()});
-	t.is(body, 'such=wow');
 });
 
 test('content-type header is not overriden when object in options.body', async t => {
-	const {body} = await got(`${s.url}/headers`, {
+	const {body: headers} = await got(`${s.url}/headers`, {
 		headers: {
 			'content-type': 'doge'
 		},
@@ -140,7 +118,15 @@ test('content-type header is not overriden when object in options.body', async t
 		},
 		json: true
 	});
-	t.is(body['content-type'], 'doge');
+	t.is(headers['content-type'], 'doge');
+});
+
+test('throws when json body is not a plain object', async t => {
+	await t.throws(got(`${s.url}`, {body: '{}', json: true}), TypeError);
+});
+
+test('throws when form body is not a plain object', async t => {
+	await t.throws(got(`${s.url}`, {body: 'such=wow', form: true}), TypeError);
 });
 
 test.after('cleanup', async () => {