Repository navigation
Fix scanner counting the leading BOM as a column when encoding is explicit - #347
Closed
afonsojanu wants to merge 1 commit into
Closed
afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
…licit When the input encoding is set explicitly via yaml_parser_set_encoding(), yaml_parser_determine_encoding() (which strips a BOM during auto-detection without touching mark.column) is skipped, so the BOM reaches the scanner instead. yaml_parser_scan_to_next_token() then calls SKIP(), which bumps mark.column along with the buffer position, so the rest of the first line ends up one column further right than it should be. A root-level block mapping terminates its first entry at the wrong spot once the line wraps, and parsing fails with "did not find expected <document start>". Per YAML 1.2 section 5.2 the BOM isn't part of the document content, so skipping it shouldn't move mark.column at all. This mirrors what the auto-detection path already does. Reported in #334, originally surfaced through Ruby's Psych binding, which always sets the encoding explicitly and so hit this on every BOM-prefixed document.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #334.
When the encoding is set explicitly with
yaml_parser_set_encoding(),yaml_parser_determine_encoding()is skipped entirely, so the BOM auto-detection path (which strips a leading BOM without touchingmark.column) never runs. The BOM then reaches the scanner, whereyaml_parser_scan_to_next_token()callsSKIP()on it.SKIP()bumpsmark.columnalong with the buffer position, so the rest of the first line ends up starting one column further right than it should.That off-by-one is enough to break block mapping detection on the first line: a root-level
key: valuepair parses fine, but the mapping is considered closed as soon as the line wraps, and parsing then fails with "did not find expected ".Per YAML 1.2 section 5.2 the BOM isn't part of the document content, so this skip shouldn't move
mark.columnat all - which is exactly what the auto-detection path already does for a BOM it strips. This fix does the same thing at the scanner level.Verified with a small standalone reproduction: parsing
"\xEF\xBB\xBFa: b\nc: d\n"with the encoding explicitly set toYAML_UTF8_ENCODING. On unmodified master this fails after the first pair with "did not find expected "; with the fix applied all four scalars parse correctly. Confirmed the failure disappears specifically because of this change by reverting it locally and re-running (stash test). Ran the existing CMake test suite (version/reader/nesting) plus the parser test suite against a handful of BOM and non-BOM documents under ASan/UBSan, no failures or sanitizer reports.Originally reported via Ruby's Psych, which always sets the encoding explicitly and so hit this on every BOM-prefixed YAML document.