Skip to content
This repository has been archived by the owner on Feb 25, 2019. It is now read-only.

Deterministic JSON Stringify #37

Open
EternalDeiwos opened this issue Aug 24, 2017 · 3 comments
Open

Deterministic JSON Stringify #37

EternalDeiwos opened this issue Aug 24, 2017 · 3 comments

Comments

@EternalDeiwos
Copy link
Member

A recurring problem that has been seen with regards to hashes and signatures is when properties get switched around between JSON.stringify calls which results in hashes not matching and signatures not verifying successfully despite semantically identical objects.

For example, an object { alg: 'RS256', kid: 'abcd' } could be stringified as:

{"alg":"RS256","kid":"abcd"}

or

{"kid":"abcd","alg":"RS256"}

The result of the stringify operation depends on the original order of the properties in the JSON string (my understanding is that the order is preserved) and the order in which properties are added to the object.

My recommendation is that we replace our stringify calls with our own deterministic stringify function, or one that we import such as json-stable-stringify.

@dmitrizagidulin
Copy link
Member

That's a really good idea. (i'd vote for json-stable-stringify)

@christiansmith
Copy link
Member

I don't doubt there are cases where this has been a problem, and have often wondered if we'd have to squash bugs caused thusly. In practice, it hasn't bitten me personally.

One reason for this may be the way modinha and json-document schemas are traversed to read/copy data when initializing. That would minimize the sort of property reordering you're talking about. In fact, it may be a deterministic function on it's own.

I could see such schemes causing problems interoperating with JSON data stringified in the "usual" way.

Just for posterity, can you point to a concrete case or two where JSON.stringify() over the same state has resulted in two different values?

@EternalDeiwos
Copy link
Member Author

@christiansmith take a look at:

https://github.com/anvilresearch/jose/blob/master/test/jose/JWDSpec.js#L24-L40

Swap lines 34 and 35 and watch the world burn.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants