Skip to content

Commit

Permalink
[New] stringify: throw on cycles, instead of an infinite loop
Browse files Browse the repository at this point in the history
Fixes #367.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Johan <johan@pop-os.localdomain>
  • Loading branch information
Johan and ljharb committed Mar 17, 2021
1 parent 586f029 commit 63766c2
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .eslintrc
Expand Up @@ -10,7 +10,7 @@
"id-length": [2, { "min": 1, "max": 25, "properties": "never" }],
"indent": [2, 4],
"max-lines-per-function": [2, { "max": 150 }],
"max-params": [2, 14],
"max-params": [2, 15],
"max-statements": [2, 52],
"multiline-comment-style": 0,
"no-continue": 1,
Expand Down
17 changes: 14 additions & 3 deletions lib/stringify.js
@@ -1,5 +1,6 @@
'use strict';

var getSideChannel = require('side-channel');
var utils = require('./utils');
var formats = require('./formats');
var has = Object.prototype.hasOwnProperty;
Expand Down Expand Up @@ -68,9 +69,15 @@ var stringify = function stringify(
format,
formatter,
encodeValuesOnly,
charset
charset,
sideChannel
) {
var obj = object;

if (sideChannel.has(object)) {
throw new RangeError('Cyclic object value');
}

if (typeof filter === 'function') {
obj = filter(prefix, obj);
} else if (obj instanceof Date) {
Expand Down Expand Up @@ -129,6 +136,7 @@ var stringify = function stringify(
? typeof generateArrayPrefix === 'function' ? generateArrayPrefix(prefix, key) : prefix
: prefix + (allowDots ? '.' + key : '[' + key + ']');

sideChannel.set(object, true);
pushToArray(values, stringify(
value,
keyPrefix,
Expand All @@ -143,7 +151,8 @@ var stringify = function stringify(
format,
formatter,
encodeValuesOnly,
charset
charset,
sideChannel
));
}

Expand Down Expand Up @@ -237,6 +246,7 @@ module.exports = function (object, opts) {
objKeys.sort(options.sort);
}

var sideChannel = getSideChannel();
for (var i = 0; i < objKeys.length; ++i) {
var key = objKeys[i];

Expand All @@ -257,7 +267,8 @@ module.exports = function (object, opts) {
options.format,
options.formatter,
options.encodeValuesOnly,
options.charset
options.charset,
sideChannel
));
}

Expand Down
5 changes: 4 additions & 1 deletion package.json
Expand Up @@ -29,6 +29,9 @@
"engines": {
"node": ">=0.6"
},
"dependencies": {
"side-channel": "^1.0.4"
},
"devDependencies": {
"@ljharb/eslint-config": "^17.5.1",
"aud": "^1.1.4",
Expand All @@ -55,7 +58,7 @@
"tests-only": "nyc tape 'test/**/*.js'",
"posttest": "aud --production",
"readme": "evalmd README.md",
"postlint": "eclint check * lib/* test/*",
"postlint": "eclint check * lib/* test/* !dist/*",
"lint": "eslint lib/*.js test/*.js",
"dist": "mkdirp dist && browserify --standalone Qs lib/index.js > dist/qs.js"
},
Expand Down
25 changes: 24 additions & 1 deletion test/stringify.js
Expand Up @@ -433,7 +433,7 @@ test('stringify()', function (t) {
st.end();
});

t.test('doesn\'t blow up when Buffer global is missing', function (st) {
t.test('does not blow up when Buffer global is missing', function (st) {
var tempBuffer = global.Buffer;
delete global.Buffer;
var result = qs.stringify({ a: 'b', c: 'd' });
Expand All @@ -442,6 +442,29 @@ test('stringify()', function (t) {
st.end();
});

t.test('does not crash when parsing circular references', function (st) {
var a = {};
a.b = a;

st['throws'](
function () { qs.stringify({ 'foo[bar]': 'baz', 'foo[baz]': a }); },
RangeError,
'cyclic values throw'
);

var circular = {
a: 'value'
};
circular.a = circular;
st['throws'](
function () { qs.stringify(circular); },
RangeError,
'cyclic values throw'
);

st.end();
});

t.test('selects properties when filter=array', function (st) {
st.equal(qs.stringify({ a: 'b' }, { filter: ['a'] }), 'a=b');
st.equal(qs.stringify({ a: 1 }, { filter: [] }), '');
Expand Down

0 comments on commit 63766c2

Please sign in to comment.