-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
[json.exception.type_error.317] cannot serialize binary data to text JSON #2067
Comments
I saw that the dump function have the It do not work however since at line 14763 and also at other place like 14774, the However this will also not allow us to save and reload the json object isn't it? I don't need the binary type: |
This looks like a bug. I will check. Can you provide your input as a quick example to trigger the issue? |
Also at line 17726 and 17730
I added the 0 just before the
I need some time to generate a short example to reproduce the issue. |
Here is minimal example:
The output should be something like:
|
As the whole binary feature has not yet been released, we are free to experiment with the behavior here. We may want to replace the
What do you think? |
(Good catch, I could confirm the bug and start a branch with a fix. Meanwhile, we can discuss #2067 (comment)) |
Seem not a bad idea! But what annoy me is the default settings if they are the following two:
I have many place where I do a To my understanding whatever is the JSON object, a call to the default Otherwise I need to know wether the current JSON contains binary to call
|
I understand your concern on the default value. Let's put that aside and think about the possible values. Let's then choose the default. Furthermore, I would like to get @OmnipotentEntity 's ideas on this as they came up with the original implementation and may share some insights in the choice of the default value. |
At first I was just thinking if a new The Then it should be the function wich transform EDIT: |
|
Also I have another cosmetic bug when pretty printing binary:
As you can see a new line is inserted in the begining of the array. At line 14895 It try to add a new line but it seem the condition is not good, for the first byte. Can be changed to:
EDIT: EDIT2: |
Ugh, how embarrassing about that serialization bug. The code is apparently in a rougher state than I thought. I'm still away from my computer on vacation, so I can't get a patch in until at least mid-next week. So... my original thoughts about serialization of binary values is, frankly, there's no good way to do it in a way that everyone will agree with. Various ideas were thrown around: integer array, base64 string, optionally prefixed or not with a The Probably a better way of implementing it would be to take an optional binary value formatting function along with the serializer and a matching binary value reading function along with the deserializer. I can leave in the Anyway, I seem to have chosen a horrible time to take a vacation, and I apologize. I also apologize for missing this thread for a few days. I'll have a patch to you as soon as I am able. |
@OmnipotentEntity No need to apologize! You did the heavy lifting of adding a binary type in the first place! It's in the develop branch now and we can spend some time polishing it and finding a nice API for it. I think I'll push the fix for passing down Please enjoy your vacation! |
The original issue is now fixed. |
Thanks for taking care of that. Thinking a little bit on the Is this a good place to hash this out? Or is there a better one? (RFC process?) |
I thought about this topic a bit:
What do you think? |
I'm in favour of the enum solution at the current state. Maybe the enum can have meaning not only for Also I want to bring some problem I'm facing:
For now I'm only aware that those data was embeded when I later call
Regards. |
FYI, I am currently working on #2082 to add missing tests and thereby make some minor refactorings or fixes to the code for binary values. Note the public API may still change as the feature is not released yet. Once I am done, I will be happy to discuss the PR. |
Ok, I'm finally back. So it sounds like there are two major wants for the binary API:
For the first, utility functions would be useful to automatically walk the JSON tree and perform these actions, but my understanding is in principle, this should be possible already. I'm not sure if that should or should not be in the library. You could make a similar (but less robust) argument regarding all sorts of carry-along data that might be serialized with the data you want. @nlohmann you seem to favor an enum based approach from a limited list of options. But I'm really not so sure that's flexible enough. This is the same interface used for general serialization to JSON, and different applications might have different requirements for their packed binary data, which is why I initially recommended passing a serialization function (and having some example/default serialization functions available). That being said, I'm not certain that dumping binary values to JSON has much utility outside of debugging, so if the reasoning behind a simple enum is because few people would use it anyway, then point taken. It's also not as if these two options are mutually exclusive. It's entirely possible to have some default functions enumerated, and have the ability to pass a general serialization function via an overload. |
I polished the binary types, see #2125. I decided against putting too much logics into the JSON serialization - I implemented a simple serialization as object like {"bytes": [1,2,3], "subtype": 42} or {"bytes": [1,2,3], "subtype": null} if no subtype is given. The result is valid JSON without throwing an exception. @OmnipotentEntity @xamix Please take a look! From my side, this would be the last remaining PR before we can release 3.8.0. |
|
#include <iostream>
#include "json.hpp"
using json = nlohmann::json;
int main()
{
std::vector<std::uint8_t> v = {{0x81, 0xA4, 0x64, 0x61, 0x74, 0x61, 0xC4, 0x0F, 0x33, 0x30, 0x30, 0x32, 0x33, 0x34, 0x30, 0x31, 0x30, 0x37, 0x30, 0x35, 0x30, 0x31, 0x30}};
json j = json::from_msgpack(v);
std::cout << j.dump(2) << std::endl;
} Output: {
"data": {
"bytes": [51, 48, 48, 50, 51, 52, 48, 49, 48, 55, 48, 53, 48, 49, 48],
"subtype": null
}
} The binary value is of type |
@nlohmann Thank you very much for all your answer! Concerning 4):
In your example, maybe subtype hould be Concerning 5): As of today with current library it throw on data which contain binary because it is not possible to save it as text and then parse data back and showing the exactly contained fields i.e: If I use the dump function with the new future release, now it generate some non existing fields in the json (in your example So now with the new release, I must parse completely the received messagepack data, Another solution is that I will implement a dump() function which will output binary data as a simple array of integer (no subtype or bytes fields), an so I will be able to also save binary data as text. Maybe something to specialize the dump behavior is possible like pseudo code below:
|
The bin8, bin16, and bin32 are no subtypes, but just different versions of the bin family which differ in the number of bytes. The difference is not related to the content. I understand your usecase, but when you receive data that you have no control over, you must process it anyway. Why would it be better to have binary data as array of integers or text rather than the current solution? |
The user/interpreter expect to see only existing fields:
With dump, it create other fields:
So there is no way to determinate that data is only an array after using dump(). And if I create an object wich is exactly the same format you dump binary, I cannot determine if data was an binaray array or a full existing object:
|
So in that case, you need to write a cleanup function that replaces binary values by the byte vector: #include <iostream>
#include "json.hpp"
using json = nlohmann::json;
template<class UnaryFunction>
void recursive_apply(json& j, UnaryFunction f)
{
for(auto it = j.begin(); it != j.end(); ++it)
{
if (it->is_structured())
{
recursive_apply(*it, f);
}
else
{
f(*it);
}
}
}
int main()
{
json j;
j[0]["desc"] = "an integer";
j[0]["data"] = 1;
j[1]["desc"] = "a binary";
j[1]["data"] = json::binary({1,2,3});
j[2]["desc"] = "an object";
j[2]["data"] = {{"foo", "bar"}};
std::cout << j << std::endl;
recursive_apply(j, [](json& j) {
if (j.is_binary()) {
// replace binary value by integer vector
j = static_cast<std::vector<std::uint8_t>>(j.get_binary());
}
});
std::cout << j << std::endl;
} Output:
|
What a nice idea! As a side note, thank you very much for your library :-) |
Closed by merging #2125. |
Describe what you want to achieve.
I need to get a string representation of my JSON object containing binary type
Describe what you tried.
Use function
dump
on my JSON objectDescribe which system (OS, compiler) you are using.
Linux Ubuntu GCC 9
Describe which version of the library you are using (release version, develop branch).
Current develop d9d1279
In my current application, I receive MessagePack serialized object containing binary array type (0xC4)
In the current develop I saw that you added handling of this
The convertion from MessagePack data to json is working:
But using
j.dump()
or stream it intostd::cout
is not possible and throw the following exception:[json.exception.type_error.317] cannot serialize binary data to text JSON
However I need to save the JSON and also print it in a LOG.
I understand that it cannot be serialized because the
binary
type would be "lost" when saved as an array and readed back since in the json format nothing can differenciate it from basic array.Type will change to
value_t::array
but at least I can save and print my object.Do you have a solution?
The text was updated successfully, but these errors were encountered: