Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An issue about JSON.stringify #725

Closed
NWU-NISL opened this issue Jun 23, 2020 · 4 comments · Fixed by #876
Closed

An issue about JSON.stringify #725

NWU-NISL opened this issue Jun 23, 2020 · 4 comments · Fixed by #876
Labels
bug Issues considered a bug

Comments

@NWU-NISL
Copy link

NWU-NISL commented Jun 23, 2020

Version

rhino-1.7.12

Test case
var NISLFuzzingFunc=function(nislMutationParameter1){
    var a={"0":1, "1":2, "2":3, "name":4};
    var b=JSON.stringify(a, nislMutationParameter1); 
    print(b);
}; 
var nislMutationArgument1=["0", "1", "name", "name"]; 
NISLFuzzingFunc(nislMutationArgument1);
Execution steps

java -jar rhino/rhino-1.7.12.jar -debug -version 200 testcase.js

Output
 {"name":4,"name":4}
Expected behavior
{"0":1,"1":2,"name":4}
Description

The JSON.stringify(value[, replacer[, space]]) method converts a JavaScript object or value to a JSON string, optionally including only the specified properties if a replacer array is specified, and the ES standard specifies that the output attributes should not contain duplicate attributes. When executing this test case, rhino has two problems: the first is that the attribute named by numeric characters cannot be output, and the second is to repeatedly output the same attribute.

contributor: @Wen Yi

@gbrail
Copy link
Collaborator

gbrail commented Jul 1, 2020

By the way -- I appreciate all the tricky but important bugs you are finding. (I also appreciate @rbri for fixing them!)

Who or what is "NWU NISL?" Are you a professional, a student, a research project... I'm just curious what methodology you are using to find these.

@YiWen-y
Copy link

YiWen-y commented Jul 14, 2020

We are a Chinese research group, and we use fuzz testing to find bugs of JS engines.

@tom08zehn
Copy link

I found a similar problem with the replacer argument but with slightly different output.

I'm using Rhino with ES6 (language version 200) but also tested with lower version 160.

I experienced a bug in Rhino with JSON.stringify() function when you provide an array of strings as 2nd argument that is used as a whitelist for the property names that you want to keep and if the property names are numbers as strings (stringified numbers). You can see a full code example here and compare Rhino result with any other JS engine (browser):

https://js.do/code/rhino-issue-object-keys-stringnumber-keys

Summary:

var obj1 = {"2":"b","3":"c","1":"a"};
JSON.stringify(obj1, ["1","2","3"]); // returns {} in Rhino but works in JS returning {"1":"a","2":"b","3":"c"}
JSON.stringify(obj1, [1,2,3]) // returns {"1.0":"a","2":"b","3":"c"} in Rhino but works in JS returning  {"1":"a","2":"b","3":"c"}

It seems like Rhino treats object property names that are stringified numbers as float types instead of strings and as a result the comparison against an array of strings fails. In addition, a comparison against an array of integer numbers succeeds... at least almost because the result is an object having property names of type "stringified floats".

@tom08zehn
Copy link

Just figured out that the trick using an array of integers instead of an array of strings also has a bug for object property name "1" (one). As shown above this returns a stringified float.

This doesn't happen for stringified numbers other than one: JSON.stringify({"2":"b","3":"c"},[2,3]));

But anyway this trick is a no-go...

tonygermano added a commit to tonygermano/rhino that referenced this issue Apr 30, 2021
- per spec, Numbers in an array replacer should be converted to strings
  prior to adding to the property list
- Changed some List objects to Collections because that's is sufficient for
  how they are being used
- This allowed the property list to be changed to a LinkedHashSet, which
  satisfies the requirements for not adding duplicate items to the list
  and retains insert order
- Uses the proper index when getting properties with integer keys
tonygermano added a commit to tonygermano/rhino that referenced this issue Apr 30, 2021
- Per spec, Numbers in an array replacer should be converted to strings
  prior to adding to the property list
- Duplicates should be removed during construction of the property list
- Changed some variables declared as List  to Collection instead because
  that is sufficient for how they are being used
- Moved List to array conversion from happening in every iteration of
  the jo method to being called once in the stringify method
- Convert strings items representing integers to their Integer value for
  correct property lookup
tonygermano added a commit to tonygermano/rhino that referenced this issue May 4, 2021
- Per spec, Numbers in an array replacer should be converted to strings
  prior to adding to the property list
- Duplicates should be removed during construction of the property list
- Changed some variables declared as List  to Collection instead because
  that is sufficient for how they are being used
- Moved List to array conversion from happening in every iteration of
  the jo method to being called once in the stringify method
- Convert strings items representing integers to their Integer value for
  correct property lookup
gbrail pushed a commit that referenced this issue May 5, 2021
- Per spec, Numbers in an array replacer should be converted to strings
  prior to adding to the property list
- Duplicates should be removed during construction of the property list
- Changed some variables declared as List  to Collection instead because
  that is sufficient for how they are being used
- Moved List to array conversion from happening in every iteration of
  the jo method to being called once in the stringify method
- Convert strings items representing integers to their Integer value for
  correct property lookup
@p-bakker p-bakker added the bug Issues considered a bug label Oct 13, 2021
@p-bakker p-bakker added this to the Release 1.7.14 milestone Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues considered a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants