Repository: sindresorhus/got (ts)

Issue:
JSON option when posting data
Would it be possible to use the `json: true` option when posting data to set `Content-Type` to `application/json` and stringify the body with `JSON.stringify`?


Pull request:
Change stringifying options behavior
Changes the behavior of the stringifying options.

* [x] don't accept plain object unless `form` or `json` is true
* [x] stringify if `json` is true
* [x] stringify if `form` is true
* [x] stringify as querystring and parse `res` as JSON if `form` and `json` are true

Things I changed because I found the code hard to deal with, and confusing without.
* clean up input validation using Sindre's suggestion from #212.
* simpler use of `body` variable. It's a shorthand for the body passed by the user. `opts.body` is what we mutate into the body we want to send.
* 'throws on bad input' test is maybe overkill

As always details are in the commit messages.

Sidenotes: I went for minimal change based on the proposal from #174. I think this can be quite a bit cleaner still. I'm considering a second PR. Specifically what I feel could be better:
* make 'body is string' case explicit instead of implicit fallback
* use early returns to limit code paths and sharpen reduce redundant input checking
* some tests use destructuring others don't _for the exact same response property_.

---
Fixes #174 

Patch:
diff --git a/index.js b/index.js
index b7cab05..6da03f9 100644
--- a/index.js
+++ b/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) {
@@ -28,6 +29,11 @@ function requestAsEventEmitter(opts) {
 	let redirectUrl;
 
 	const get = opts => {
+		if (opts.protocol !== 'http:' && opts.protocol !== 'https:') {
+			ee.emit('error', new got.UnsupportedProtocolError(opts));
+			return;
+		}
+
 		const fn = opts.protocol === 'https:' ? https : http;
 
 		const req = fn.request(opts, res => {
@@ -208,11 +214,13 @@ function normalizeArguments(url, opts) {
 
 	opts = Object.assign(
 		{
-			protocol: 'http:',
 			path: '',
 			retries: 5
 		},
 		url,
+		{
+			protocol: url.protocol || 'http:' // Override both null/undefined with default protocol
+		},
 		opts
 	);
 
@@ -236,31 +244,37 @@ function normalizeArguments(url, opts) {
 		opts.headers.accept = 'application/json';
 	}
 
-	let body = opts.body;
-
-	if (body) {
-		if (typeof body !== 'string' && !(body !== null && typeof body === 'object')) {
-			throw new Error('options.body must be a ReadableStream, string, Buffer or plain Object');
+	const body = opts.body;
+	if (body !== null && body !== undefined) {
+		const headers = opts.headers;
+		if (!isStream(body) && typeof body !== 'string' && !Buffer.isBuffer(body) && !(opts.form || opts.json)) {
+			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()}`;
-		} else if (body !== null && typeof body === 'object' && !Buffer.isBuffer(body) && !isStream(body)) {
-			opts.headers['content-type'] = opts.headers['content-type'] || 'application/x-www-form-urlencoded';
-			body = querystring.stringify(body);
-			opts.body = body;
+			headers['content-type'] = headers['content-type'] || `multipart/form-data; boundary=${body.getBoundary()}`;
+		} else if (opts.form && isPlainObj(body)) {
+			headers['content-type'] = headers['content-type'] || 'application/x-www-form-urlencoded';
+			opts.body = querystring.stringify(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)) {
-			const length = typeof body === 'string' ? Buffer.byteLength(body) : body.length;
-			opts.headers['content-length'] = length;
+		if (headers['content-length'] === undefined && headers['transfer-encoding'] === undefined && !isStream(body)) {
+			const length = typeof opts.body === 'string' ? Buffer.byteLength(opts.body) : opts.body.length;
+			headers['content-length'] = length;
 		}
-	}
 
-	opts.method = (opts.method || 'GET').toUpperCase();
+		opts.method = (opts.method || 'POST').toUpperCase();
+	} else {
+		opts.method = (opts.method || 'GET').toUpperCase();
+	}
 
 	if (opts.hostname === 'unix') {
 		const matches = /(.+):(.+)/.exec(opts.path);
@@ -364,4 +378,9 @@ got.MaxRedirectsError = createErrorClass('MaxRedirectsError', function (statusCo
 	this.message = 'Redirected 10 times. Aborting.';
 });
 
+got.UnsupportedProtocolError = createErrorClass('UnsupportedProtocolError', function (opts) {
+	stdError.call(this, {}, opts);
+	this.message = `Unsupported protocol "${opts.protocol}"`;
+});
+
 module.exports = got;
diff --git a/package.json b/package.json
index 2d22b56..e2b09a1 100644
--- a/package.json
+++ b/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",
diff --git a/readme.md b/readme.md
index 2a0877e..708750b 100644
--- a/readme.md
+++ b/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
 
@@ -200,6 +213,10 @@ When server response code is not 2xx. Contains `statusCode` and `statusMessage`.
 
 When server redirects you more than 10 times.
 
+#### got.UnsupportedProtocolError
+
+When given an unsupported protocol.
+
 
 ## Proxies
 


Test patch:
diff --git a/test/arguments.js b/test/arguments.js
index a772a57..dd24306 100644
--- a/test/arguments.js
+++ b/test/arguments.js
@@ -24,12 +24,8 @@ test.before('setup', async () => {
 });
 
 test('url is required', async t => {
-	try {
-		await got();
-		t.fail('Exception was not thrown');
-	} catch (err) {
-		t.regex(err.message, /Parameter `url` must be a string or object, not undefined/);
-	}
+	const err = await t.throws(got());
+	t.regex(err.message, /Parameter `url` must be a string or object, not undefined/);
 });
 
 test('options are optional', async t => {
@@ -65,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();
 });
diff --git a/test/headers.js b/test/headers.js
index 3eb8b38..247ade7 100644
--- a/test/headers.js
+++ b/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',
-		json: true
+		body: 'sup'
 	})).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,
-		json: true
+		body: form
 	})).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, {
-		body: form,
-		json: true
+	const body = (await got(s.url, {
+		body: form
 	})).body;
+	const headers = JSON.parse(body);
 	t.is(headers['content-type'], `multipart/form-data; boundary=${form.getBoundary()}`);
 });
 
diff --git a/test/http.js b/test/http.js
index 6b0c997..ad9985c 100644
--- a/test/http.js
+++ b/test/http.js
@@ -67,6 +67,11 @@ test('status code 304 doesn\'t throw', async t => {
 	t.is(response.body, '');
 });
 
+test('invalid protocol throws', async t => {
+	const err = await t.throws(got('c:/nope.com', {json: true}));
+	t.is(err.constructor, got.UnsupportedProtocolError);
+});
+
 test('buffer on encoding === null', async t => {
 	const data = (await got(s.url, {encoding: null})).body;
 	t.truthy(Buffer.isBuffer(data));
diff --git a/test/json.js b/test/json-parse.js
similarity index 86%
rename from test/json.js
rename to test/json-parse.js
index ddc0dae..1e2c98b 100644
--- a/test/json.js
+++ b/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();
 });
diff --git a/test/post.js b/test/post.js
index fe8d5e0..035d8c9 100644
--- a/test/post.js
+++ b/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`, {
-		body: 'wow',
-		json: true
-	});
-	t.is(body['content-length'], '3');
+	const {body} = await got(`${s.url}/headers`, {body: 'wow'});
+	const headers = JSON.parse(body);
+	t.is(headers['content-length'], '3');
 });
 
 test('content-length header with Buffer body', async t => {
-	const {body} = await got(`${s.url}/headers`, {
-		body: Buffer.from('wow'),
-		json: true
-	});
-	t.is(body['content-length'], '3');
+	const {body} = await got(`${s.url}/headers`, {body: Buffer.from('wow')});
+	const headers = JSON.parse(body);
+	t.is(headers['content-length'], '3');
 });
 
 test('content-length header with Stream body', async t => {
-	const {body} = await got(`${s.url}/headers`, {
-		body: intoStream(['wow']),
-		json: true
-	});
-	t.is(body['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
-	t.is(body['content-length'], undefined);
+	const {body} = await got(`${s.url}/headers`, {body: intoStream(['wow'])});
+	const headers = JSON.parse(body);
+	t.is(headers['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
+	t.is(headers['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');
-	t.is(body['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');
+	const headers = JSON.parse(body);
+	t.is(headers['transfer-encoding'], 'chunked', 'likely failed to get headers at all');
+	t.is(headers['content-length'], undefined);
 });
 
 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 () => {