From 1d07fce635848f6c7056d826b03b1c96901469a2 Mon Sep 17 00:00:00 2001 From: Mihaela M Date: Sat, 27 Jun 2026 15:53:16 +0200 Subject: [PATCH] fix(parsing): reject a comment glued to earlier content on its line A '#' that begins a comment must be preceded by whitespace or open the line; glued to earlier same-line content (e.g. "value"# c, [ a, b ]#c, >#c, a flow entry then ,#c) it is neither a comment nor, in those positions, plain-scalar content, so the document is malformed. Such input was previously accepted as a comment. A '#' that opens a line, follows a space, or sits inside a plain scalar (a#b, where it stays literal content and never reaches this check) is unaffected. The scanner tracks whether a real token has been scanned on the current physical line, reset on each line break, rather than comparing token line numbers: the streaming tokenizer drains its token buffer between scans, and a block scalar's blank line emits an empty scalar attributed to the next line, either of which would misread as content. Measured against the official YAML test-suite, parse-level conformance rises from 243/333 (73.0%) to 247/333 (74.2%): four wrongly-accepted invalid documents are now rejected, with no valid document regressing. The scalar-scanning routines move to Scanner.Scalars.swift to keep the file within the length limit. --- CHANGELOG.md | 9 ++ Sources/Parsing/ParseError.swift | 11 +- Sources/Parsing/Scanner.Scalars.swift | 154 +++++++++++++++++++++++ Sources/Parsing/Scanner.swift | 173 +++----------------------- Sources/Parsing/TokenCursor.swift | 1 + Tests/CommentWhitespaceTests.swift | 35 ++++++ 6 files changed, 228 insertions(+), 155 deletions(-) create mode 100644 Sources/Parsing/Scanner.Scalars.swift create mode 100644 Tests/CommentWhitespaceTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index b6a66f2..929d3f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A `#` glued to earlier content on its line (e.g. `"value"# c`, `[ a, b ]#c`, `>#c`) is now + rejected: the spec requires a comment to be preceded by whitespace or to open a line, and such + input was previously accepted as a comment. A `#` that opens a line, follows a space, or sits + inside a plain scalar (`a#b`, where it is literal content) is unaffected. The check tracks + whether real content has been scanned on the current line, rather than comparing token line + numbers, because the streaming tokenizer drains its buffer and a block scalar's blank line + emits an empty scalar attributed to the next line. Lifts official test-suite parse-level + conformance from 243/333 (73.0%) to 247/333 (74.2%); four wrongly-accepted invalid documents + are now correctly rejected with no valid document regressing. - A block scalar followed by a less-indented comment line no longer fails with a spurious "unexpected indentation". When a comment ends a block scalar, its own column was processed as a dedent and, landing on a width no indentation level held, raised an error; a comment's diff --git a/Sources/Parsing/ParseError.swift b/Sources/Parsing/ParseError.swift index 12b0f3d..fe018cd 100644 --- a/Sources/Parsing/ParseError.swift +++ b/Sources/Parsing/ParseError.swift @@ -18,6 +18,7 @@ public extension PureYAML.Parsing { case unexpectedEvent(expected: String, actual: String, line: Int, column: Int) case unexpectedToken(expected: String, actual: String, line: Int, column: Int) case nestingTooDeep(line: Int, column: Int) + case commentNotPrecededByWhitespace(line: Int, column: Int) public var description: String { switch self { @@ -55,6 +56,8 @@ public extension PureYAML.Parsing { "expected \(expected) at line \(line), column \(column), found \(actual)" case let .nestingTooDeep(line, column): "nesting is too deep at line \(line), column \(column)" + case let .commentNotPrecededByWhitespace(line, column): + "a comment must be preceded by whitespace at line \(line), column \(column)" } } @@ -94,6 +97,8 @@ public extension PureYAML.Parsing { "unexpectedToken" case .nestingTooDeep: "nestingTooDeep" + case .commentNotPrecededByWhitespace: + "commentNotPrecededByWhitespace" } } @@ -116,7 +121,8 @@ public extension PureYAML.Parsing { let .undefinedAlias(_, line, _), let .unexpectedEvent(_, _, line, _), let .unexpectedToken(_, _, line, _), - let .nestingTooDeep(line, _): + let .nestingTooDeep(line, _), + let .commentNotPrecededByWhitespace(line, _): line } } @@ -140,7 +146,8 @@ public extension PureYAML.Parsing { let .undefinedAlias(_, _, column), let .unexpectedEvent(_, _, _, column), let .unexpectedToken(_, _, _, column), - let .nestingTooDeep(_, column): + let .nestingTooDeep(_, column), + let .commentNotPrecededByWhitespace(_, column): column } } diff --git a/Sources/Parsing/Scanner.Scalars.swift b/Sources/Parsing/Scanner.Scalars.swift new file mode 100644 index 0000000..b23a422 --- /dev/null +++ b/Sources/Parsing/Scanner.Scalars.swift @@ -0,0 +1,154 @@ +extension PureYAML.Parsing.Scanner { + func scanComment(_ state: inout State) { + let start = state.reader.mark + state.reader.advance() + if state.reader.peek() == " " { + state.reader.advance() + } + let value = state.reader.consume { character in + !isLineBreak(character) + } + state.append(.comment(value), mark: start, endMark: state.reader.mark) + } + + func scanQuotedScalar( + _ style: PureYAML.Parsing.ScalarStyle, + state: inout State, + ) throws { + let start = state.reader.mark + let quote: Character = style == .doubleQuoted ? "\"" : "'" + state.reader.advance() + var value = "" + var previousWasEscape = false + while let character = state.reader.peek() { + if style == .singleQuoted, character == quote, state.reader.peek(offset: 1) == quote { + value.append(character) + state.reader.advance() + value.append(character) + state.reader.advance() + continue + } + if character == quote, !previousWasEscape { + break + } + value.append(character) + previousWasEscape = character == "\\" && style == .doubleQuoted && !previousWasEscape + state.reader.advance() + if character != "\\" { + previousWasEscape = false + } + } + guard state.reader.peek() == quote else { + throw PureYAML.Parsing.ParseError.unterminatedQuotedString(line: start.line) + } + state.reader.advance() + // Unlike a plain scalar (which consumes the space before a `:` into itself), a quoted scalar + // stops at its closing quote, so the reader sits at any spaces before a `:`. A space between + // an implicit key and its mapping-value indicator is allowed, so look past them. + if isAtMappingValueIndicatorAfterSpaces(state: state) { + state.append(.mappingKey, mark: start, endMark: start) + } + state.append(.scalar(value: value, style: style), mark: start, endMark: state.reader.mark) + } + + func scanBlockScalarHeader( + _ style: PureYAML.Parsing.BlockScalarStyle, + state: inout State, + ) { + let start = state.reader.mark + var chomping = PureYAML.Parsing.BlockScalarChomping.clip + var indentationIndicator: Int? + state.reader.advance() + while let character = state.reader.peek(), isBlockScalarHeaderCharacter(character) { + if character == "-" { + chomping = .strip + } else if character == "+" { + chomping = .keep + } else if character.isNumber { + indentationIndicator = Int(String(character)) + } + state.reader.advance() + } + state.append( + .blockScalarHeader(style: style, chomping: chomping), + mark: start, + endMark: state.reader.mark, + ) + state.pendingBlockScalarIndentation = true + state.pendingBlockScalarIndentationIndicator = indentationIndicator + } + + func scanBlockScalarContent(_ state: inout State) { + let start = state.reader.mark + let value = state.blockScalarContentPrefix + state.reader.consume { character in + !isLineBreak(character) + } + state.blockScalarContentPrefix = "" + state.append(.scalar(value: value, style: .plain), mark: start, endMark: state.reader.mark) + } + + func scanNamedToken( + kind: (String) -> PureYAML.Parsing.TokenKind, + missingNameError: (Int) -> PureYAML.Parsing.ParseError, + state: inout State, + ) throws { + let start = state.reader.mark + state.reader.advance() + let name = state.reader.consume { character in + !isTokenTerminator(character) + } + guard !name.isEmpty else { + throw missingNameError(start.line) + } + state.append(kind(name), mark: start, endMark: state.reader.mark) + } + + func scanTag(_ state: inout State) throws { + let start = state.reader.mark + state.reader.advance() + var value = "!" + if state.reader.peek() == "<" { + value.append(state.reader.advance() ?? "<") + var didClose = false + while let character = state.reader.peek() { + value.append(character) + state.reader.advance() + if character == ">" { + didClose = true + break + } + } + guard didClose else { + throw PureYAML.Parsing.ParseError.unterminatedTag(line: start.line) + } + } else { + value += state.reader.consume { character in + !isTokenTerminator(character) + } + value = expandTag(value, state: state) + } + state.append(.tag(value), mark: start, endMark: state.reader.mark) + } + + func scanPlainScalar(_ state: inout State) { + let start = state.reader.mark + var value = "" + while let character = state.reader.peek() { + if shouldEndPlainScalar(character, state: state, accumulated: value) { + break + } + value.append(character) + state.reader.advance() + } + var trimmed = trimTrailingSpaces(value) + let isMappingKey = appendMappingKeyIfNeeded(start: start, state: &state) + if !isMappingKey { + trimmed = scanPlainScalarContinuations( + after: trimmed, + firstLineIndent: start.column - 1, + state: &state, + ) + } + state.append(.scalar(value: trimmed, style: .plain), mark: start, endMark: state.reader.mark) + } +} diff --git a/Sources/Parsing/Scanner.swift b/Sources/Parsing/Scanner.swift index 9881984..a47cfea 100644 --- a/Sources/Parsing/Scanner.swift +++ b/Sources/Parsing/Scanner.swift @@ -11,6 +11,7 @@ extension PureYAML.Parsing { state.reader.advance() captureBlockScalarBlankLine(&state) state.isAtLineStart = true + state.sawContentOnLine = false continue } @@ -55,6 +56,12 @@ extension PureYAML.Parsing.Scanner { var pendingBlockScalarLeadingBlankLines = 0 var blockScalarIndentation: Int? var blockScalarContentPrefix = "" + // Whether a real token has been scanned on the current physical line. Reset when the scan + // loop consumes a line break and set as each token is scanned, so the scanner can tell a + // comment that opens a line (valid) from one glued to earlier content on it (invalid). A + // simple flag, not a token-line comparison, because a blank line inside a block scalar + // emits an empty scalar attributed to the next line, which would read as phantom content. + var sawContentOnLine = false var tagHandles = [ "!": "!", "!!": "tag:yaml.org,2002:", @@ -93,7 +100,9 @@ extension PureYAML.Parsing.Scanner { extension PureYAML.Parsing.Scanner { func scanToken(_ state: inout State) throws { + let columnBeforeSeparation = state.reader.mark.column skipSeparation(&state) + let consumedSeparation = state.reader.mark.column > columnBeforeSeparation guard let character = state.reader.peek(), !isLineBreak(character) else { return } @@ -102,6 +111,17 @@ extension PureYAML.Parsing.Scanner { scanBlockScalarContent(&state) return } + // A `#` begins a comment only at the start of a line's content or after whitespace. Glued to + // earlier same-line content (e.g. `"x"#c`, `]#c`, `>#c`) it is not a plain-scalar character + // either, so the document is malformed; a plain scalar's own `#` is consumed before reaching + // here, so this rejects only the genuinely invalid case. + if character == "#", !consumedSeparation, state.sawContentOnLine { + throw PureYAML.Parsing.ParseError.commentNotPrecededByWhitespace( + line: state.reader.mark.line, + column: state.reader.mark.column, + ) + } + state.sawContentOnLine = true if try scanLineIndicatorToken(character, state: &state) { return } @@ -244,157 +264,4 @@ extension PureYAML.Parsing.Scanner { state.flowContexts[state.flowContexts.count - 1].isExpectingKey = false } } - - func scanComment(_ state: inout State) { - let start = state.reader.mark - state.reader.advance() - if state.reader.peek() == " " { - state.reader.advance() - } - let value = state.reader.consume { character in - !isLineBreak(character) - } - state.append(.comment(value), mark: start, endMark: state.reader.mark) - } - - func scanQuotedScalar( - _ style: PureYAML.Parsing.ScalarStyle, - state: inout State, - ) throws { - let start = state.reader.mark - let quote: Character = style == .doubleQuoted ? "\"" : "'" - state.reader.advance() - var value = "" - var previousWasEscape = false - while let character = state.reader.peek() { - if style == .singleQuoted, character == quote, state.reader.peek(offset: 1) == quote { - value.append(character) - state.reader.advance() - value.append(character) - state.reader.advance() - continue - } - if character == quote, !previousWasEscape { - break - } - value.append(character) - previousWasEscape = character == "\\" && style == .doubleQuoted && !previousWasEscape - state.reader.advance() - if character != "\\" { - previousWasEscape = false - } - } - guard state.reader.peek() == quote else { - throw PureYAML.Parsing.ParseError.unterminatedQuotedString(line: start.line) - } - state.reader.advance() - // Unlike a plain scalar (which consumes the space before a `:` into itself), a quoted scalar - // stops at its closing quote, so the reader sits at any spaces before a `:`. A space between - // an implicit key and its mapping-value indicator is allowed, so look past them. - if isAtMappingValueIndicatorAfterSpaces(state: state) { - state.append(.mappingKey, mark: start, endMark: start) - } - state.append(.scalar(value: value, style: style), mark: start, endMark: state.reader.mark) - } - - func scanBlockScalarHeader( - _ style: PureYAML.Parsing.BlockScalarStyle, - state: inout State, - ) { - let start = state.reader.mark - var chomping = PureYAML.Parsing.BlockScalarChomping.clip - var indentationIndicator: Int? - state.reader.advance() - while let character = state.reader.peek(), isBlockScalarHeaderCharacter(character) { - if character == "-" { - chomping = .strip - } else if character == "+" { - chomping = .keep - } else if character.isNumber { - indentationIndicator = Int(String(character)) - } - state.reader.advance() - } - state.append( - .blockScalarHeader(style: style, chomping: chomping), - mark: start, - endMark: state.reader.mark, - ) - state.pendingBlockScalarIndentation = true - state.pendingBlockScalarIndentationIndicator = indentationIndicator - } - - func scanBlockScalarContent(_ state: inout State) { - let start = state.reader.mark - let value = state.blockScalarContentPrefix + state.reader.consume { character in - !isLineBreak(character) - } - state.blockScalarContentPrefix = "" - state.append(.scalar(value: value, style: .plain), mark: start, endMark: state.reader.mark) - } - - func scanNamedToken( - kind: (String) -> PureYAML.Parsing.TokenKind, - missingNameError: (Int) -> PureYAML.Parsing.ParseError, - state: inout State, - ) throws { - let start = state.reader.mark - state.reader.advance() - let name = state.reader.consume { character in - !isTokenTerminator(character) - } - guard !name.isEmpty else { - throw missingNameError(start.line) - } - state.append(kind(name), mark: start, endMark: state.reader.mark) - } - - func scanTag(_ state: inout State) throws { - let start = state.reader.mark - state.reader.advance() - var value = "!" - if state.reader.peek() == "<" { - value.append(state.reader.advance() ?? "<") - var didClose = false - while let character = state.reader.peek() { - value.append(character) - state.reader.advance() - if character == ">" { - didClose = true - break - } - } - guard didClose else { - throw PureYAML.Parsing.ParseError.unterminatedTag(line: start.line) - } - } else { - value += state.reader.consume { character in - !isTokenTerminator(character) - } - value = expandTag(value, state: state) - } - state.append(.tag(value), mark: start, endMark: state.reader.mark) - } - - func scanPlainScalar(_ state: inout State) { - let start = state.reader.mark - var value = "" - while let character = state.reader.peek() { - if shouldEndPlainScalar(character, state: state, accumulated: value) { - break - } - value.append(character) - state.reader.advance() - } - var trimmed = trimTrailingSpaces(value) - let isMappingKey = appendMappingKeyIfNeeded(start: start, state: &state) - if !isMappingKey { - trimmed = scanPlainScalarContinuations( - after: trimmed, - firstLineIndent: start.column - 1, - state: &state, - ) - } - state.append(.scalar(value: trimmed, style: .plain), mark: start, endMark: state.reader.mark) - } } diff --git a/Sources/Parsing/TokenCursor.swift b/Sources/Parsing/TokenCursor.swift index 8b814cb..c4579dd 100644 --- a/Sources/Parsing/TokenCursor.swift +++ b/Sources/Parsing/TokenCursor.swift @@ -90,6 +90,7 @@ extension PureYAML.Parsing { scannerBox.state.reader.advance() scanner.captureBlockScalarBlankLine(&scannerBox.state) scannerBox.state.isAtLineStart = true + scannerBox.state.sawContentOnLine = false continue } diff --git a/Tests/CommentWhitespaceTests.swift b/Tests/CommentWhitespaceTests.swift new file mode 100644 index 0000000..2f91afb --- /dev/null +++ b/Tests/CommentWhitespaceTests.swift @@ -0,0 +1,35 @@ +@testable import PureYAML +import Testing + +@Suite("Comments must be preceded by whitespace") +struct CommentWhitespaceTests { + @Test("A comment glued to earlier content on its line is rejected", arguments: [ + "key: \"value\"# comment\n", // after a quoted scalar + "[ a, b ]#comment\n", // after a flow sequence close + "{ a: b }#comment\n", // after a flow mapping close + "block: >#comment\n x\n", // after a block scalar header + "[ a,#comment\n]\n", // after a flow entry separator + ]) + func test_gluedCommentIsRejected(_ yaml: String) throws { + #expect(throws: PureYAML.Parsing.ParseError.self) { + try PureYAML.parse(yaml) + } + } + + @Test("A comment preceded by whitespace or opening a line is accepted", arguments: [ + "key: \"value\" # comment\n", // a space before the comment + "# comment\nkey: v\n", // a comment opening the document + "a: 1\n # comment\nb: 2\n", // an indented comment line + "[ a,\n # comment\n b ]\n", // a comment line inside a flow collection + ]) + func test_wellSpacedCommentIsAccepted(_ yaml: String) throws { + _ = try PureYAML.parse(yaml) + } + + @Test("A '#' inside a plain scalar stays literal content, not a comment") + func test_hashInsidePlainScalar() throws { + #expect(try PureYAML.parse("key: a#b\n") == .mapping(.init([ + .init(key: "key", value: .string("a#b")), + ]))) + } +}