From 5dafb435d8946805cbb25bccb1bfac31139915d1 Mon Sep 17 00:00:00 2001 From: Igor Kalashnikov Date: Sat, 20 Feb 2016 22:23:51 +0300 Subject: querystring: using toString for objects on querystring.escape This commit fixes an inconsistency in querystring.escape objects handling compared to native encodeURIComponent function. Fixes: https://github.com/nodejs/node/issues/5309 PR-URL: https://github.com/nodejs/node/pull/5341 Reviewed-By: James M Snell Reviewed-By: Brian White --- lib/querystring.js | 8 ++++++-- test/parallel/test-querystring-escape.js | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-querystring-escape.js diff --git a/lib/querystring.js b/lib/querystring.js index ad1b5861a0..bacfc4bd70 100644 --- a/lib/querystring.js +++ b/lib/querystring.js @@ -90,8 +90,12 @@ for (var i = 0; i < 256; ++i) QueryString.escape = function(str) { // replaces encodeURIComponent // http://www.ecma-international.org/ecma-262/5.1/#sec-15.1.3.4 - if (typeof str !== 'string') - str += ''; + if (typeof str !== 'string') { + if (typeof str === 'object') + str = String(str); + else + str += ''; + } var out = ''; var lastPos = 0; diff --git a/test/parallel/test-querystring-escape.js b/test/parallel/test-querystring-escape.js new file mode 100644 index 0000000000..e15ce3266d --- /dev/null +++ b/test/parallel/test-querystring-escape.js @@ -0,0 +1,24 @@ +'use strict'; +require('../common'); +const assert = require('assert'); + +const qs = require('querystring'); + +assert.deepEqual(qs.escape(5), '5'); +assert.deepEqual(qs.escape('test'), 'test'); +assert.deepEqual(qs.escape({}), '%5Bobject%20Object%5D'); +assert.deepEqual(qs.escape([5, 10]), '5%2C10'); + +// using toString for objects +assert.strictEqual( + qs.escape({test: 5, toString: () => 'test', valueOf: () => 10 }), + 'test' +); + +// toString is not callable, must throw an error +assert.throws(() => qs.escape({toString: 5})); + +// should use valueOf instead of non-callable toString +assert.strictEqual(qs.escape({toString: 5, valueOf: () => 'test'}), 'test'); + +assert.throws(() => qs.escape(Symbol('test'))); -- cgit v1.2.3