fktn-k / fkYAML

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

LF / CR LF files are not handled correctly outside Windows #375

Closed sndth closed 1 month ago

sndth commented 1 month ago

Description

when you set EOL as CR LF (I didn't checked CR) in YAML file and you try to parse it on e.g. Linux, you will get error: type_error: The node value is not a sequence. type=mapping

but when you try to parse LF file on Windows, everything should work

Reproduction steps

  1. create CR LF file (just by creating on Windows)
  2. copy file to Linux (server or something)
  3. when you try to deserialize document - node value is not a sequence!

Expected vs. actual results

expected result should handle EOL (for example like JSON libraries) and correctly parse document currently we have problem with parsing

Minimal code example

No response

Error messages

No response

Compiler and operating system

Linux: C++latest, Clang 14.0.6 Windows: C++latest, MSVC latest

Library version

0.3.9/0.3.10

Validation

fktn-k commented 1 month ago

@sndth
Thanks for filing the issue.
I'll try the reproduction steps and see if something wrong happens in my environment too.

fktn-k commented 1 month ago

@sndth
I successfully reproduce your situation.
After investigating the root cause, a bug was found in normalizing new line codes (either LF or CR+LF) in a given input into LF only, which has been implemented to reduce too many if-else branches due to taking care of both LF and CR+LF in the whole deserialization process.
But I somehow dropped checks for the normalized input values for the UTF-8 encoding and unfortunately didn't recognize the bug until now.

I've already fixed it in my local environment, and so the fix will be available soon.
Sorry for your inconvenience.

fktn-k commented 1 month ago

@sndth
The fix is merged into the develop branch since all the CI checks passed successfully.
Can you confirm the fix also works in your environment too?

sndth commented 1 month ago

Yes, fix works like charm, thank you very much for fast support!

fktn-k commented 1 month ago

@sndth
Glad to hear that :) Thanks for your fast reaction too!
Feel free to reopen this issue if you noticed anything related to this issue but still unresolved.