Skip to content
Permalink
Browse files

querystring: using toString for objects on querystring.escape

This commit fixes an inconsistency in querystring.escape objects handling
compared to native encodeURIComponent function.

Fixes: #5309
PR-URL: #5341
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
  • Loading branch information...
silentroach authored and jasnell committed Feb 20, 2016
1 parent 59d23ad commit 5dafb435d8946805cbb25bccb1bfac31139915d1
Showing with 30 additions and 2 deletions.
  1. +6 −2 lib/querystring.js
  2. +24 −0 test/parallel/test-querystring-escape.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;

@@ -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')));

0 comments on commit 5dafb43

Please sign in to comment.
You can’t perform that action at this time.