-
Notifications
You must be signed in to change notification settings - Fork 65
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 base64 parse doesn't do what it is supposed to #1326
Comments
thanks for finding this reproducer. |
How did you create this example? The string usually should include the null term char. |
I stole from here and modified: |
Here's a better example: std::string RC_CINEMA_WEB = R"xyzxyz(
{
"schema":
{
"index.html": {"dtype":"char8_str","number_of_elements": 1,"offset": 0,"stride": 1,"element_bytes": 1,"endianness": "little"},
"cvlib": {"dtype":"char8_str","number_of_elements": 1,"offset": 1,"stride": 1,"element_bytes": 1,"endianness": "little"},
"cvlib2": {"dtype":"char8_str","number_of_elements": 1,"offset": 2,"stride": 1,"element_bytes": 1,"endianness": "little"}
},
"data":
{
"base64": "ab123"
}
}
)xyzxyz";
conduit::Node res;
res.parse(RC_CINEMA_WEB,"conduit_base64_json");
std::cout << res.to_yaml() << std::endl;
|
Strings usually should include the null term char. |
When i see number of elements ==1, there maybe an expectation that is a null termed string. Could be why our other cases are wrong, or it could mean this case is ill posed. |
How do I put the null term char into base64 representation to test? |
I would argue that conduit should only read number_of_elements elements instead of reading until it hits a null terminator |
gives you
Both should be a single character long. index.html is picking up the character from cvlib.
The text was updated successfully, but these errors were encountered: