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

byte string support #483

Closed
izvit opened this issue Mar 3, 2017 · 19 comments
Closed

byte string support #483

izvit opened this issue Mar 3, 2017 · 19 comments
Assignees
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON kind: enhancement/improvement solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@izvit
Copy link

izvit commented Mar 3, 2017

I was wondering if there were any plans for supporting CBOR byte strings (RFC 7049 type 2)?

@tksatware
Copy link

JSON is a subset of CBOR and does not support binary data.
If you need to rely on binary data in a cbor stream, this library is not the right tool.

@nlohmann
Copy link
Owner

nlohmann commented Mar 3, 2017

RFC 7049 states in Sect. 4.1:

o A byte string (major type 2) that is not embedded in a tag that
specifies a proposed encoding is encoded in base64url without
padding and becomes a JSON string.

I have to look deeper into this, but this does not sound like arbitrary binary data. What do you think @tksatware ?

@tksatware
Copy link

CBOR provides a tag as a HINT how the binary data should be decoded. Example "this is a string, but base64 encoded".

I would not go down that route.

First, the json library would need to support the different decoding algorithms and second, converting between json/cbor would not be bijective as it is when using only JSON features.

The concept of tags in CBOR is also not available in JSON (as said, JSON is a subset).

@nlohmann
Copy link
Owner

nlohmann commented Mar 8, 2017

@izvit What's your opinion on this?

@izvit
Copy link
Author

izvit commented Mar 10, 2017

In general, it's a perfectly valid argument; however, it's unclear to me what the actual motivation for any CBOR support in the library.
If the use case is compatibility with other CBOR libraries, as tksaware correctly pointed out, the JSON subset alone would not be enough.
I would imagine most developers choose a binary format for message size or performance reasons, and as far as I know, the CBOR subset that has been chosen has not been standardized, which means it would only be useful for project solely using this library. Thus, adding support for any CBOR data type that could easily be converted to JSON via hints, and provide value to users seems like a reasonable option.

@nlohmann
Copy link
Owner

nlohmann commented Mar 10, 2017

Hm. I should have another look at Section 4 ("Converting Data between CBOR and JSON") of RFC 7049. It would be good to follow the standard.

@tksatware
Copy link

The Standard is non-normative here. It does not comment on the bijectivity, it suggests to convert byte streams to base64 encoded JSON strings. The other way round, it has no determined outcome.

@nlohmann
Copy link
Owner

@tksatware You are right - I read Section 4 and did not really felt the need to change the library's behavior. However, I need to improve the documentation on the supported subset of CBOR to manage the expectations in this regard.

@izvit
Copy link
Author

izvit commented Mar 14, 2017

CBOR standard can't possible address the bijectivity, since they are two different standards; however, there is no good reason why that bijectivity can't be guaranteed by the implementation. All that would do is expand the supported subset of CBOR for users that are looking for byte string support, while adding useful encoder/decoder functionally for the JSON only users.

@nlohmann
Copy link
Owner

But what about other unsupported types?

@izvit
Copy link
Author

izvit commented Mar 14, 2017

I guess it goes back to my original question - what was the motivation for the JSON library to support any subset of CBOR?

@nlohmann
Copy link
Owner

The motivation was to be able to serialize any JSON value to CBOR and to read such serializations. It was always clear that not all types could be supported, and the library throws an exception when an unsupported type is encountered.

@izvit
Copy link
Author

izvit commented Mar 14, 2017

That was the implementation, but doesn't quite answer what was the use case for such functionality. Why does the user need partial CBOR support? It wouldn't allow arbitrary CBOR input, which break compatibility. So I would think it has to be a performance argument. In that case, expanding the supported CBOR subset to include byte string support seems to make a lot of sense.

@izvit
Copy link
Author

izvit commented Mar 14, 2017

BTW. "No, byte strings will not be supported" is a perfectly valid answer. Thank you for sharing this great project with the community!

@izvit izvit closed this as completed Mar 14, 2017
@nlohmann
Copy link
Owner

So it's really about the byte strings? (I mean, you are OK with the way I treat datetime, etc.?)

@izvit
Copy link
Author

izvit commented Mar 14, 2017

I think other types can addressed, but then you risk becoming a CBOR library that supports JSON rather than the other way around.

@izvit
Copy link
Author

izvit commented Mar 14, 2017

@nlohmann To answer your question, binary payloads can provide serious performance gains for certain applications, and adding support would allow for those applications to seamlessly switch between JSON and CBOR without major hassle.

@tksatware
Copy link

@izvit actually, I replaced my own cbor encoder/decoder in a specific project by this library, it allows me to support the compact CBOR as well as JSON (for debugging) when communicating with an HTTP server.

It was a 5 minutes replacement with nlohmann's library! For the types supported, it is (nearly complete) bijective.

@nlohmann nlohmann added the aspect: binary formats BSON, CBOR, MessagePack, UBJSON label Mar 28, 2017
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Apr 16, 2020
@nlohmann nlohmann self-assigned this Apr 16, 2020
@nlohmann nlohmann added this to the Release 3.8.0 milestone Apr 16, 2020
@nlohmann
Copy link
Owner

Byte strings are now supported, see #1662.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON kind: enhancement/improvement solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants