fktn-k / fkYAML

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

Parse error #347

Closed adjbcbc closed 4 months ago

adjbcbc commented 4 months ago

Description

it seems that an issue occurs when parsing yaml in the following file.

contexts:                                                                    
- context:                                                                   
    cluster: abcdef                                                          
    extension:                                                               
    - extension:                                                             
        last-update: blah                                                    
        version: 0.1.0                                                       
      name: blah                                                             
    bug: default                                                             
  ctx: ctx    

Reproduction steps

 #include <iostream>                                                     
 #include "fkYAML/node.hpp"                                              
 int main() {                                                            
     fkyaml::node root = fkyaml::node::deserialize("./yamltest");        

     for (const auto &ctx : root["contexts"]) {
         std::cout << ctx["context"]["cluster"].get_value<std::string>(); // error                  
         std::cout << ctx["context"]["bug"].get_value<std::string>();      // error
     }                                                                   
 }                

Expected vs. actual results

terminate called after throwing an instance of 'fkyaml::v0_3_7::type_error' what(): type_error: The target node is neither of sequence nor mapping types. type=null Aborted (core dumped)

Minimal code example

No response

Error messages

No response

Compiler and operating system

g++ 14.1.1 & linux 6.9.1-arch1-1

Library version

v0.3.7

Validation

fktn-k commented 4 months ago

@adjbcbc
Thanks for sharing the issue and sorry for your inconvenience.

First of all, the fkyaml::node::deserialize() function doesn't expect a file path as an input.
If you want to deserialize file contents, you must first open a file and pass the file handle, i.e., FILE* or any object which can be interpreted as std::istream, like the following:

#include <iostream>
#include <fstream> // ADDED
#include "fkYAML/node.hpp"
int main() {                                                       
    std::ifstream ifs("./yamltest");                    // ADDED
    fkyaml::node root = fkyaml::node::deserialize(ifs); // CHANGED

    for (const auto &ctx : root["contexts"]) {
        std::cout << ctx["context"]["cluster"].get_value<std::string>(); // error
        std::cout << ctx["context"]["bug"].get_value<std::string>();      // error
    }
}

If a file path is given, fkYAML tries to parse the string ./yamltest itself, which is why you encountered the reported error.

So, I tried the input with the above code and found a bug thanks to your report.
There seems to be a bug in handling indentation after a block sequence with the same indentation as its key node (line 9 in the YAML file). Due to the bug, the node associated with the bug key was mistakenly parsed as a child of the sequence node with extension key.

Although I'm going to fix the bug hopefully this weekend, a quick workaround is to use the slightly modified version which works fine in my environment:

contexts:                                                                    
- context:                                                                   
    cluster: abcdef                                                          
    extension:                                                               
      - extension:                                                             
          last-update: blah                                                    
          version: 0.1.0                                                       
        name: blah                                                             
    bug: default                                                             
  ctx: ctx    
fktn-k commented 4 months ago

@adjbcbc
The PR #348 has fixed the bug in parsing the YAML snippet you shared above.
Could you confirm if you can now get the correct parse result on the latest develop branch?

adjbcbc commented 4 months ago

thinks for quick fix i have confirmed that above example has been corrected properly. but, i originally intended to try following file, but now encounted the following error.

terminate called after throwing an instance of 'fkyaml::v0_3_7::parse_error'
  what():  parse_error: invalid flow mapping ending is found. (at line 26, column 14)
Aborted (core dumped)
apiVersion: v1
clusters:
- cluster:
    certificate-authority: /home/x/.minikube/ca.crt
    extensions:
    - extension:
        last-update: Mon, 27 May 2024 06:05:41 UTC
        provider: minikube.sigs.k8s.io
        version: v1.32.0
      name: cluster_info
    server: https://192.168.30.12:8441
  name: minikube
contexts:
- context:
    cluster: minikube
    extensions:
    - extension:
        last-update: Mon, 27 May 2024 06:05:41 UTC
        provider: minikube.sigs.k8s.io
        version: v1.32.0
      name: context_info
    namespace: default
    user: minikube
  name: minikube
current-context: minikube
kind: Config
preferences: {}
users:
- name: minikube
  user:
    client-certificate: /home/x/.minikube/profiles/minikube/client.crt
    client-key: /home/x/.minikube/profiles/minikube/client.key

how can i solve the above issue? tools like kubectl had no problems reading that file

fktn-k commented 4 months ago

@adjbcbc
Thanks for confirming the fix and reporting the new issue.

The above error was caused because I forgot to implement for empty flow containers in the PR #350.
I can't think of a workaround for this issue, but it shouldn't require any major changes and will be fixed soon.

fktn-k commented 4 months ago

The error on empty flow containers has been fixed in the PR #351.
And I've checked that the parsing of the YAML file shared here succeeds with the modified parser, just locally though.
Can you please check if it can now be parsed with no issue in your environment too?

adjbcbc commented 4 months ago

i have confirmed that it works in my environment as well. thank you for the quick response. this is a good project!