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

revisit canonical json and non-integer numbers #413

Closed
carolynvs opened this issue May 4, 2021 · 1 comment · Fixed by #414
Closed

revisit canonical json and non-integer numbers #413

carolynvs opened this issue May 4, 2021 · 1 comment · Fixed by #414
Assignees

Comments

@carolynvs
Copy link
Contributor

I am confused about the official spec for canonical json and whether or not it supports non-integer numbers. I think this is the spec and they do say that big/high precision numbers should be stored as strings and parsed in the consumer. They do not rule out all non-integer numbers though.

That spec also says that this go implementation is conformant: https://github.com/cyberphone/json-canonicalization/tree/master/go/src/webpki.org/jsoncanonicalizer

We are currently using docker's canonical json library which does not support non-integer numbers. It has a hard check when marshaling a float that it is a whole number.

I have tried the other library and it round trips the value "1.5" (which is what I have users asking for) fine.

When we changed the spec to exclude non-integer numbers that decision was based on a old?bad? copy of the canonical json spec (I was looking at the spec referenced by the docker library http://wiki.laptop.org/go/Canonical_JSON) instead of the one above. I now believe that was a mistake and that we should revert that change and update cnab-go to use a different library.

Tagging @chris-crone and @technosophos to weigh in.

@carolynvs
Copy link
Contributor Author

Got the 👍 from Chris to update us to use this other library, which is closer to implementing the full spec.

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

Successfully merging a pull request may close this issue.

1 participant