saphyr-rs / saphyr

A set of crates dedicated to parsing YAML.
58 stars 5 forks source link

Error parsing nested block strings #13

Closed Tate-CC closed 1 month ago

Tate-CC commented 1 month ago

I'm hitting an issue when trying to parse the content:

---
array:
  - object:
      array:
        - object:
            array:
              - text: >-
                  Line 1
                  Line 2
---

The call to saphyr::YamlLoader::load_from_str returns an error did not find expected key at byte 140 line 9 column 19 even though the content validates as yaml when using an external validator.

It seems like it may be related to the level of nesting when hitting the block string as the following content does parse successfully:

---
array:
  - object:
    array:
      - object:
          array:
            - text: >-
                Line 1
                Line 2
---

(Notice the first level array has been unindented by one level)

I can also reproduce the error using literal style block instead of a folded style one, and without the chomping indicator.

Ethiraric commented 1 month ago

Can you check if this happens on the master branch?

The following test passes on my computer:

#[test]
fn test_issue13() {
    let s = r"---
array:
  - object:
      array:
        - object:
            array:
              - text: >-
                  Line 1
                  Line 2
...";

    assert_eq!(
        run_parser(s).unwrap(),
        [
            Event::StreamStart,
            Event::DocumentStart(true),
            Event::MappingStart(0, None),
            Event::Scalar("array".to_string(), TScalarStyle::Plain, 0, None),
            Event::SequenceStart(0, None),
            Event::MappingStart(0, None),
            Event::Scalar("object".to_string(), TScalarStyle::Plain, 0, None),
            Event::MappingStart(0, None),
            Event::Scalar("array".to_string(), TScalarStyle::Plain, 0, None),
            Event::SequenceStart(0, None),
            Event::MappingStart(0, None),
            Event::Scalar("object".to_string(), TScalarStyle::Plain, 0, None),
            Event::MappingStart(0, None),
            Event::Scalar("array".to_string(), TScalarStyle::Plain, 0, None),
            Event::SequenceStart(0, None),
            Event::MappingStart(0, None),
            Event::Scalar("text".to_string(), TScalarStyle::Plain, 0, None),
            Event::Scalar("Line 1 Line 2".to_string(), TScalarStyle::Folded, 0, None),
            Event::MappingEnd,
            Event::SequenceEnd,
            Event::MappingEnd,
            Event::MappingEnd,
            Event::SequenceEnd,
            Event::MappingEnd,
            Event::MappingEnd,
            Event::SequenceEnd,
            Event::MappingEnd,
            Event::DocumentEnd,
            Event::StreamEnd
        ]
    );
}
Tate-CC commented 1 month ago

Hi @Ethiraric 👋

It does look like it's working on master 🎉

Thanks for checking this out! Do you have any idea when we can expect a new release to crates with the fix? For the moment we can pin our version to the specific git revision, but we won't be able to publish until there's a new numbered release.

Ethiraric commented 1 month ago

I need to consolidate tests a bit before I release. I don't expect it to last past the end of this week.

Ethiraric commented 1 month ago

I withdraw the above comment. I hadn't seen the fuzzer PR and it is reporting a fair amount of issues.

Do you urgently need a release?

Tate-CC commented 1 month ago

No, not urgently. We've got our workaround in place, so no rush!

Ethiraric commented 1 month ago

I'm looking to update both saphyr and saphyr-parser, and it doesn't seem possible as of yet. I'm cc'ing @davvid in hope they know more than I do.

Previously, running the yaml-test-suite with saphyr-parser used yaml-rust2. This is something I have changed so that it now uses saphyr (uncovering a bug in the parser during the journey).

This however means that now we have

# In `saphyr`
[dependencies]
saphyr-parser = "0.0.3" # It appears I need to set the version explicitly in order to publish
# In `saphyr-parser`
[dev-dependencies]
saphyr = "0.0.3

Cargo is very unhappy with this. I can't seem to publish two packages at once, so both crates depend on a version of the other crate that isn't yet published. I tried downgrading saphyr-parser's dependency on saphyr to 0.0.1, but then tests do not compile.

Should I revert my change and keep using yaml-rust2?

Ethiraric commented 1 month ago

I don't know what I did differently today, but I could publish the parser. 0.0.3 is on crates.io.