search

douglascrockford / JSON-js

JSON in JavaScript
http://www.JSON.org/
8.7k stars 4.59k forks source link

False positives when detecting cyclic stucture, this time with correct source file naming #136

Closed vsigler closed 1 year ago

vsigler commented 1 year ago

To correct https://github.com/douglascrockford/JSON-js/issues/135, this time submitting with the correct file names in the stacktrace to make everyone involved happy. :)

Not sure if it is not intentional, but the cyclic structure detection seems to be too simplified. Instead of checking only within the same stack, it throws an error even when there is the same object used anywhere within the object structure. See the following example:

var aa = { "test": "tst" };
var bb = { "one" : aa, "two": aa};
JSON.stringify(bb)

Produces:

Uncaught TypeError: Converting circular structure to JSON
    at str (json2.js:324:23)
    at str (json2.js:390:29)
    at JSON.stringify (json2.js:471:20)
    at test.html:9:6

The object is prefectly fine to be serialized into json and has no cyclic structures. I would say the detection should only consider actual stack as it is traversing the serialized object.

douglascrockford commented 1 year ago

I get bug reports for code that I did not write, or for obsolete code that was put on npm by someone else decades ago and not kept up to date. When you bitched about a file in a language that I do not use, I had to assume it was one of those.

I accepted an unsolicited pull request seven months ago that introduced your bug. I have reverted to the previous edition, five years old.

I hope that you are happy too.

vsigler commented 1 year ago

Thanks and no worries, I have never maintained anything public (or not for long anyway). I understand that after a while, one gets their shields up.