fktn-k/fkYAML

Output stream conversion of nodes lacks quoting/escape of string values

Closed this issue · 5 comments

bobziuchkovski commented

Description

While testing fkYAML, I noticed the output stream conversion of fkyaml::node string values lack quoting, which can change the YAML doc's semantics.

Example: a string value of "1" should be output with quoting or else it becomes an integral value in the output, which changes the semantics.

Reproduction steps

Parse and then output the following YAML doc:

Input doc:

stuff:
  - id: '1'
    name: Foo

fkYAML output representation (string scalar value becomes an integer)

stuff:
  -
    id: 1
    name: Foo

Expected vs. actual results

Expected: output representation retains semantic equality of input doc.
Actual: output representation changes semantic equality of input doc.

Minimal code example

std::ifstream ifs("test.yaml");
fkyaml::node root = fkyaml::node::deserialize(ifs);
std::cout << root << std::endl;

Error messages

No response

Compiler and operating system

Clang on MacOS 13.6

Library version

develop HEAD

Validation

fktn-k commented

@bobziuchkovski
Thanks for sharing the issue.
I just realized the serialization feature does nothing to avoid such cases (currently it just works like a simple extension of the std::to_string function).
It must have some functionalities to preserve semantic equality between an input and an output.

  • 👍1
fktn-k commented

@bobziuchkovski
Trying to fix the issues reported here on this branch.
I think most of the related issues have already been resolved.

fktn-k commented

@bobziuchkovski
The PR which fixes the issue related to missing quotes/escapes in serialized string values, has been merged.
I think the reported issues have been fixed and you can test it by checking out the HEAD of the develop branch.

bobziuchkovski commented

@fktn-k confirmed! That addresses everything I encountered on my end. HEAD of develop is working great. Thanks!

  • ❤️1
fktn-k commented

@bobziuchkovski
Thanks! It's good to hear that:D
I'll close this issue then.