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

dag-json vs. json: is json trully json? #470

Closed
hacdias opened this issue Oct 10, 2022 · 2 comments
Closed

dag-json vs. json: is json trully json? #470

hacdias opened this issue Oct 10, 2022 · 2 comments

Comments

@hacdias
Copy link
Member

hacdias commented Oct 10, 2022

Hi!

We're considering supporting actual json and cbor data formats in Kubo (ref). As I was testing if JSON worked properly, I run into the following:

~ ❯ ipfs dag put --input-codec json --store-codec json
ipfs: Reading from /dev/stdin; send Ctrl-d to stop.
{ 10: "hello" }
Error: unexpected int token while expecting map key

After looking at the code, it seems that the codec json actually uses the codec dag-json for decoding (and encoding), which means that json doesn't support many of the features of real JSON.

// Decode deserializes data from the given io.Reader and feeds it into the given datamodel.NodeAssembler.
// Decode fits the codec.Decoder function interface.
//
// This is the function that will be registered in the default multicodec registry during package init time.
func Decode(na datamodel.NodeAssembler, r io.Reader) error {
return dagjson.DecodeOptions{
ParseLinks: false,
ParseBytes: false,
}.Decode(na, r)
}

Is this a bug, or not? If not, why? If yes, are there any plans for fixing this?

@rvagg
Copy link
Member

rvagg commented Oct 10, 2022

It's not so much that json uses dag-json, it's that they both share the underlying engine which happens to have a mode whereby it can interpret {"/":"..."} as links and {"/":{"bytes":"..."}} as bytes.

It's not going to yield the same flexibility as "proper" JSON because the IPLD data model has some other constraints - and you're running into one of them here - map keys can only be strings, which arguably should also be true in JSON but because it's related to JavaScript we get to throw in integers too.

What we could do is allow parsing of {10:"hello"} but it'd have to be turned in to {"10":"hello"} internally.

@hacdias
Copy link
Member Author

hacdias commented Oct 11, 2022

Interesting. It makes sense. I was under the impression that JSON keys could be numbers but apparently, according to the RFC 8259 it does not:

"An object structure is represented as a pair of curly brackets surrounding zero or more name/value pairs (or members). A name is a string." - https://datatracker.ietf.org/doc/html/rfc8259#section-4 (emphasis mine)

I will close this issue then. However, your suggestion of parsing 10 to "10" could, perhaps, be an improvement of life and only fail if there are multiple keys with the same string version.

@hacdias hacdias closed this as completed Oct 11, 2022
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

No branches or pull requests

2 participants