fktn-k / fkYAML

A C++ header-only YAML library
MIT License
69 stars 7 forks source link

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

Closed bobziuchkovski closed 10 months ago

bobziuchkovski commented 11 months ago

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 11 months ago

@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.

fktn-k commented 11 months ago

@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 10 months ago

@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 10 months ago

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

fktn-k commented 10 months ago

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