Yaml: only treat % at the start of a line as a directive - #8561
Merged
Conversation
A comment before the first document that contains a `%` (for example a URL with a percent-encoded character, or prose like `# 100% coverage`) was parsed as a directive, which relocated the comment and any trailing blank line on print and turned the source into a `ParseError`. `parseDirectivesFromPrefix` now applies the same start-of-line guard that `preScanDirectives` already had. `preScanDirectives` in turn no longer treats a leading comment line as document content, so a directive that follows a comment is still pre-scanned and keeps its exact text. Fixes #8560
Member
Author
|
Given the small size of the change, the readability, and test coverage, I'm fairly confident we can see this through and rely on asynchronous review if needed. |
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.
%#8560Problem
A comment before the first (implicit) document containing a
%was parsed as a directive:printed back as
Parser.requirePrintEqualsInputthen rejects the file and it becomes aParseError— so any YAML whose header comment block contains a percent-encoded URL or prose like# 100% coveragedrops out of the LST entirely. Introduced by Support for directives in Yaml #6529 (v8.72.0).Fix
YamlParser.parseDirectivesFromPrefixscanned for%anywhere in the document prefix. It now applies the same "must be at the start of a line" guard that its siblingpreScanDirectivesalready had.While testing this I found a second, related bug:
preScanDirectivestreated a leading comment line as document content and skipped past everything up to the next---, so a directive following a comment was never pre-scanned. It then fell back to reconstructing the directive from SnakeYAML'sDocumentStartEvent, whoseDumperOptions.Versionenum has no1.2— soround-tripped as
%YAML 1.1.preScanDirectivesnow accumulates a comment line into the pending directive prefix instead, keeping the directive's exact source text.Tests
Four cases added to
DirectiveTest, covering the variants from the issue (comment before a mapping, before a sequence, a comment starting with%) plus the directive-after-comment case above. All four fail onmain.