From 441ba7c4098da15cb97481194df823c2b61669e2 Mon Sep 17 00:00:00 2001 From: simov Date: Mon, 2 Feb 2015 16:09:42 +0200 Subject: [PATCH 1/2] Do not rfc3986 escape JSON bodies Example {json:{}} or {body:{}, json:true} Related to https://github.com/request/request/pull/1315 Closes https://github.com/request/request/issues/1395 --- request.js | 4 ++-- tests/test-rfc3986.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/request.js b/request.js index 93d8a5670..f43c8c6b5 100644 --- a/request.js +++ b/request.js @@ -1428,15 +1428,15 @@ Request.prototype.json = function (val) { if (self.body !== undefined) { if (!/^application\/x-www-form-urlencoded\b/.test(self.getHeader('content-type'))) { self.body = safeStringify(self.body) + } else { + self.body = rfc3986(self.body) } - self.body = rfc3986(self.body) if (!self.hasHeader('content-type')) { self.setHeader('content-type', 'application/json') } } } else { self.body = safeStringify(val) - self.body = rfc3986(self.body) if (!self.hasHeader('content-type')) { self.setHeader('content-type', 'application/json') } diff --git a/tests/test-rfc3986.js b/tests/test-rfc3986.js index 8f2be26b0..debd2d154 100644 --- a/tests/test-rfc3986.js +++ b/tests/test-rfc3986.js @@ -28,11 +28,11 @@ function runTest (t, options) { t.equal(data, 'rfc3986=%21%2A%28%29%27') } else { - t.equal(data, '{"rfc3986":"%21%2A%28%29%27"}') + t.equal(data, '{"rfc3986":"!*()\'"}') } } if (typeof options.json === 'object') { - t.equal(data, '{"rfc3986":"%21%2A%28%29%27"}') + t.equal(data, '{"rfc3986":"!*()\'"}') } res.writeHead(200) From 29f98f31ed1c90a08786e7c8345eaf0bbd289877 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Mon, 2 Feb 2015 09:15:32 -0600 Subject: [PATCH 2/2] Explicitly specify expected request body in RFC3986 test --- tests/test-rfc3986.js | 76 +++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/tests/test-rfc3986.js b/tests/test-rfc3986.js index debd2d154..cfd27f11b 100644 --- a/tests/test-rfc3986.js +++ b/tests/test-rfc3986.js @@ -20,20 +20,7 @@ function runTest (t, options) { if (options.qs) { t.equal(req.url, '/?rfc3986=%21%2A%28%29%27') } - if (options.form) { - t.equal(data, 'rfc3986=%21%2A%28%29%27') - } - if (options.body) { - if (options.headers) { - t.equal(data, 'rfc3986=%21%2A%28%29%27') - } - else { - t.equal(data, '{"rfc3986":"!*()\'"}') - } - } - if (typeof options.json === 'object') { - t.equal(data, '{"rfc3986":"!*()\'"}') - } + t.equal(data, options._expectBody) res.writeHead(200) res.end('done') @@ -51,28 +38,67 @@ function runTest (t, options) { }) } +var bodyEscaped = 'rfc3986=%21%2A%28%29%27' + , bodyJson = '{"rfc3986":"!*()\'"}' + var cases = [ - {qs: {rfc3986: '!*()\''}}, - {qs: {rfc3986: '!*()\''}, json: true}, - {form: {rfc3986: '!*()\''}}, - {form: {rfc3986: '!*()\''}, json: true}, - {qs: {rfc3986: '!*()\''}, form: {rfc3986: '!*()\''}}, - {qs: {rfc3986: '!*()\''}, form: {rfc3986: '!*()\''}, json: true}, { + _name: 'qs', + qs: {rfc3986: '!*()\''}, + _expectBody: '' + }, + { + _name: 'qs + json', + qs: {rfc3986: '!*()\''}, + json: true, + _expectBody: '' + }, + { + _name: 'form', + form: {rfc3986: '!*()\''}, + _expectBody: bodyEscaped + }, + { + _name: 'form + json', + form: {rfc3986: '!*()\''}, + json: true, + _expectBody: bodyEscaped + }, + { + _name: 'qs + form', + qs: {rfc3986: '!*()\''}, + form: {rfc3986: '!*()\''}, + _expectBody: bodyEscaped + }, + { + _name: 'qs + form + json', + qs: {rfc3986: '!*()\''}, + form: {rfc3986: '!*()\''}, + json: true, + _expectBody: bodyEscaped + }, + { + _name: 'body + header + json', headers: {'content-type': 'application/x-www-form-urlencoded; charset=UTF-8'}, body: 'rfc3986=!*()\'', - json: true + json: true, + _expectBody: bodyEscaped }, { - body: {rfc3986: '!*()\''}, json: true + _name: 'body + json', + body: {rfc3986: '!*()\''}, + json: true, + _expectBody: bodyJson }, { - json: {rfc3986: '!*()\''} + _name: 'json object', + json: {rfc3986: '!*()\''}, + _expectBody: bodyJson } ] -cases.forEach(function (options, index) { - tape('rfc3986 ' + index, function(t) { +cases.forEach(function (options) { + tape('rfc3986 ' + options._name, function(t) { runTest(t, options) }) })