fix(parsing): reject raw control characters in parse(), matching libyaml
PureYAML.parse() silently accepted raw C0 control characters anywhere in the input; Yams's underlying libyaml rejects them unconditionally at the reader level. Found by comparing parse() directly against Yams on a real-world OpenAPI corpus. The printable-character predicate this needed already existed (CharacterClass.isPrintable, matching libyaml's c-printable set) but was only ever wired into anchor-name validation, and the separate PreflightScanner control-character check is opt-in via validate(), not part of what parse() itself runs. Reader now records the index of the first non-printable scalar as it buffers input (covering both batch and streaming), and fetchNextToken() throws once scanning has actually reached that position, not merely once one exists, so an earlier real syntax error is not hidden behind a later control character. Verified with 5 new tests and a full re-run of the official yaml-test-suite conformance (333/333 parse-level, 208/208 value-level, unchanged) and the whole-suite streaming differential (still clean), confirming no regression.
This commit is contained in:
parent
41ca600d93
commit
f9fe2325f7
43
CHANGELOG.md
43
CHANGELOG.md
@ -9,6 +9,49 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Added
|
||||
|
||||
- Reject raw control characters anywhere in the input, matching libyaml's
|
||||
own reader-level behavior. Found by directly comparing `PureYAML.parse()`
|
||||
against Yams on a 221MB real-world OpenAPI/compose-file corpus (150
|
||||
files): a genuinely public file with corrupted, mojibake-looking encoding
|
||||
contains a raw C0 control character, which Yams's underlying libyaml
|
||||
rejects unconditionally at the reader level ("control characters are not
|
||||
allowed"), but `PureYAML.parse()` silently accepted and built a value tree
|
||||
from. `PureYAML` already had a printable-character predicate matching
|
||||
libyaml's `c-printable` set (`CharacterClass.isPrintable`), but it was
|
||||
only ever applied narrowly to anchor names
|
||||
(`LibYAML.Scanner.Anchor.swift`), never to the input generally; the
|
||||
`Validation.PreflightScanner`'s separate `invalidControlCharacterColumn`
|
||||
check exists but is opt-in via `validate()`, not part of what `parse()`
|
||||
itself runs.
|
||||
`Reader` now records the index of the first non-printable scalar in
|
||||
whatever it has buffered (`init`/`append(_:)`, covering both batch and
|
||||
streaming input), and `fetchNextToken()` throws once actual scanning has
|
||||
reached that position (gated on `reader.index >= badIndex`, not just "one
|
||||
exists"), so a real syntax error earlier in the document is not hidden
|
||||
behind a control character occurring later, matching the order libyaml's
|
||||
own byte-by-byte reader would surface them in. New `ParseError` case
|
||||
`.invalidControlCharacter(line:column:)`, translated from the scanner
|
||||
error the same way `.tabIndentation` already is.
|
||||
Verified with 5 new tests (a bare control character rejected at the exact
|
||||
position; a tab in scalar content, and CR/LF, both still legal, since
|
||||
`isPrintable` already correctly includes them; a control character 5000
|
||||
lines into a document still caught; an earlier real syntax error not
|
||||
masked by a later control character) plus a full re-run of the official
|
||||
yaml-test-suite conformance (still 333/333 parse-level, 208/208
|
||||
value-level of comparable cases, unchanged) and the whole-suite streaming
|
||||
differential (still clean), confirming this closes a real gap without
|
||||
regressing anything already passing. Negligible throughput impact
|
||||
measured on the existing 9.5MB benchmark fixture (within normal run-to-run
|
||||
noise).
|
||||
Disclosed limitation: the check is applied in `fetchNextToken()`, which
|
||||
`scanStep()`'s plain-scalar resume path (`resumePlainScalar`, the
|
||||
streaming API) does not route through, so a token containing a control
|
||||
character can be returned to a streaming caller one token before this
|
||||
fires. `scanAll()`/`PureYAML.parse()`, the primary target here (matching
|
||||
what the corpus comparison actually calls), always calls
|
||||
`fetchNextToken()` again before returning, so it is caught before a batch
|
||||
parse completes.
|
||||
|
||||
- Add a public `PureYAML.Streaming` namespace (issue #4), exposing the
|
||||
internal `LibYAML.Scanner.scanStep()` proven below for the first time as a
|
||||
callable public API rather than a module-internal implementation detail.
|
||||
|
||||
@ -22,9 +22,21 @@ extension PureYAML.Parsing.LibYAML {
|
||||
/// unreachable when `isFinal` is true.
|
||||
private(set) var ranDry = false
|
||||
|
||||
/// The index of the first non-`c-printable` scalar found in `scalars`
|
||||
/// (libyaml's reader-level check, `yaml_parser_update_buffer`'s printable
|
||||
/// validation), or nil if none has been found yet. Set once, by whichever
|
||||
/// of `init`/`append(_:)` first buffers the offending scalar; never
|
||||
/// cleared. `fetchNextToken()` checks this once per token and throws,
|
||||
/// mirroring libyaml rejecting raw control characters (anything outside
|
||||
/// `c-printable`, i.e. not a plain tab/CR/LF and not in the printable
|
||||
/// ranges `CharacterClass.isPrintable` implements) wherever they occur in
|
||||
/// the stream, not only inside specific constructs like anchor names.
|
||||
private(set) var invalidControlCharacterIndex: Int?
|
||||
|
||||
init(_ string: String = "", isFinal: Bool = true) {
|
||||
scalars = Array(string.unicodeScalars)
|
||||
self.isFinal = isFinal
|
||||
recordFirstInvalidControlCharacter(in: 0 ..< scalars.count)
|
||||
}
|
||||
|
||||
/// `PEEK_TOKEN` / `CHECK_AT`: the scalar `offset` ahead, or nil past the
|
||||
@ -49,8 +61,10 @@ extension PureYAML.Parsing.LibYAML {
|
||||
/// reader position instead of restoring a snapshot (`PlainScalarSuspended`)
|
||||
/// has no other point where a stale `true` would ever become false again.
|
||||
mutating func append(_ chunk: String) {
|
||||
let previousCount = scalars.count
|
||||
scalars.append(contentsOf: chunk.unicodeScalars)
|
||||
ranDry = false
|
||||
recordFirstInvalidControlCharacter(in: previousCount ..< scalars.count)
|
||||
}
|
||||
|
||||
/// Declare that no further chunks are coming: a `peek` past the buffered
|
||||
@ -90,5 +104,49 @@ extension PureYAML.Parsing.LibYAML {
|
||||
line += 1
|
||||
column = 0
|
||||
}
|
||||
|
||||
/// The 0-based line/column of `scalars[index]`, computed by counting line
|
||||
/// breaks from the start of the buffer. Not the hot `mark` path (that
|
||||
/// tracks `line`/`column` incrementally as scanning consumes characters);
|
||||
/// used only once, lazily, when `fetchNextToken()` actually needs to report
|
||||
/// `invalidControlCharacterIndex` as a `Mark`, since that index can be
|
||||
/// anywhere in the buffer, not necessarily at the reader's current
|
||||
/// scanning position.
|
||||
func mark(atIndex target: Int) -> PureYAML.Parsing.Mark {
|
||||
var line = 0
|
||||
var column = 0
|
||||
var cursor = 0
|
||||
while cursor < target, cursor < scalars.count {
|
||||
if scalars[cursor] == "\r", cursor + 1 < scalars.count, scalars[cursor + 1] == "\n" {
|
||||
line += 1
|
||||
column = 0
|
||||
cursor += 2
|
||||
} else if CharacterClass.isBreak(scalars[cursor]) {
|
||||
line += 1
|
||||
column = 0
|
||||
cursor += 1
|
||||
} else {
|
||||
column += 1
|
||||
cursor += 1
|
||||
}
|
||||
}
|
||||
return PureYAML.Parsing.Mark(line: line, column: column, index: target)
|
||||
}
|
||||
|
||||
/// Scans `range` (a slice of `scalars` just buffered by `init`/`append(_:)`)
|
||||
/// for the first scalar outside `CharacterClass.isPrintable`, recording its
|
||||
/// index if one is found and none was already recorded. Only the first
|
||||
/// occurrence overall matters (matching libyaml, which reports the first
|
||||
/// invalid byte and stops), so once `invalidControlCharacterIndex` is set
|
||||
/// this is a no-op.
|
||||
private mutating func recordFirstInvalidControlCharacter(in range: Range<Int>) {
|
||||
guard invalidControlCharacterIndex == nil else {
|
||||
return
|
||||
}
|
||||
for index in range where !CharacterClass.isPrintable(scalars[index]) {
|
||||
invalidControlCharacterIndex = index
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -23,6 +23,25 @@ extension PureYAML.Parsing.LibYAML.Scanner {
|
||||
fetchStreamStart()
|
||||
return
|
||||
}
|
||||
// Gated on `reader.index >= badIndex`, not just "is one set", so a real
|
||||
// syntax error earlier in the document (which normal scanning would reach
|
||||
// and throw first) is not hidden behind a control character that only
|
||||
// occurs later: this only fires once the reader has genuinely scanned up
|
||||
// to the offending position, matching the order libyaml's own byte-by-byte
|
||||
// reader would hit them in. `resumePlainScalar`'s own suspend/resume path
|
||||
// (`scanStep()`, the streaming API) does not route through
|
||||
// `fetchNextToken()` and so is not covered by this check; a token
|
||||
// containing a control character can be returned to a streaming caller one
|
||||
// token before this fires. `scanAll()`/`PureYAML.parse()`, the primary
|
||||
// target, always calls `fetchNextToken()` again before returning, so this
|
||||
// still surfaces before a batch parse completes.
|
||||
if let badIndex = reader.invalidControlCharacterIndex, reader.index >= badIndex {
|
||||
throw ScannerError(
|
||||
context: "while scanning for the next token",
|
||||
problem: "control characters are not allowed",
|
||||
mark: reader.mark(atIndex: badIndex),
|
||||
)
|
||||
}
|
||||
|
||||
try scanToNextToken()
|
||||
try checkFlowContinuationIndent()
|
||||
|
||||
@ -8,6 +8,8 @@ extension PureYAML.Parsing.ParseError {
|
||||
let context = error.context ?? ""
|
||||
if error.problem.hasPrefix("found a tab") {
|
||||
self = .tabIndentation(line: line)
|
||||
} else if error.problem == "control characters are not allowed" {
|
||||
self = .invalidControlCharacter(line: line, column: column)
|
||||
} else if context == "while scanning a quoted scalar", error.problem.hasSuffix("end of stream") {
|
||||
self = .unterminatedQuotedString(line: line)
|
||||
} else if context.hasPrefix("while scanning a tag") || context.hasSuffix("TAG directive") {
|
||||
|
||||
@ -3,6 +3,7 @@ public extension PureYAML.Parsing {
|
||||
enum ParseError: Swift.Error, Equatable, Sendable, CustomStringConvertible {
|
||||
case emptyDocument
|
||||
case tabIndentation(line: Int)
|
||||
case invalidControlCharacter(line: Int, column: Int)
|
||||
case unexpectedIndentation(line: Int)
|
||||
case mixedCollectionStyles(line: Int)
|
||||
case expectedMappingKey(line: Int)
|
||||
@ -30,6 +31,8 @@ public extension PureYAML.Parsing {
|
||||
"document is empty"
|
||||
case let .tabIndentation(line):
|
||||
"tabs are not allowed for indentation at line \(line)"
|
||||
case let .invalidControlCharacter(line, column):
|
||||
"invalid control character at line \(line), column \(column)"
|
||||
case let .unexpectedIndentation(line):
|
||||
"unexpected indentation at line \(line)"
|
||||
case let .mixedCollectionStyles(line):
|
||||
@ -79,6 +82,8 @@ public extension PureYAML.Parsing {
|
||||
"emptyDocument"
|
||||
case .tabIndentation:
|
||||
"tabIndentation"
|
||||
case .invalidControlCharacter:
|
||||
"invalidControlCharacter"
|
||||
case .unexpectedIndentation:
|
||||
"unexpectedIndentation"
|
||||
case .mixedCollectionStyles:
|
||||
@ -147,6 +152,7 @@ public extension PureYAML.Parsing {
|
||||
let .unexpectedEvent(_, _, line, _),
|
||||
let .unexpectedToken(_, _, line, _),
|
||||
let .nestingTooDeep(line, _),
|
||||
let .invalidControlCharacter(line, _),
|
||||
let .malformed(_, line, _):
|
||||
line
|
||||
}
|
||||
@ -176,6 +182,7 @@ public extension PureYAML.Parsing {
|
||||
let .unexpectedEvent(_, _, _, column),
|
||||
let .unexpectedToken(_, _, _, column),
|
||||
let .nestingTooDeep(_, column),
|
||||
let .invalidControlCharacter(_, column),
|
||||
let .malformed(_, _, column):
|
||||
column
|
||||
}
|
||||
|
||||
55
Tests/InvalidControlCharacterTests.swift
Normal file
55
Tests/InvalidControlCharacterTests.swift
Normal file
@ -0,0 +1,55 @@
|
||||
@testable import PureYAML
|
||||
import Testing
|
||||
|
||||
/// The reader-level control-character check (`Reader.recordFirstInvalidControlCharacter`,
|
||||
/// `fetchNextToken()`'s check in `LibYAML.Scanner.Fetch.swift`) added to close a real gap
|
||||
/// found comparing PureYAML against Yams on a real-world OpenAPI corpus: Yams's underlying
|
||||
/// libyaml rejects raw control characters (`c-printable` violations) anywhere in the input,
|
||||
/// while `PureYAML.parse()` previously accepted them silently (the only existing check,
|
||||
/// `PreflightScanner.invalidControlCharacterColumn`, is opt-in via `validate()`, not part of
|
||||
/// the core scanner `parse()` uses).
|
||||
@Suite("Invalid control character rejection")
|
||||
struct InvalidControlCharacterTests {
|
||||
@Test("A raw control character in scalar content is rejected with the correct position")
|
||||
func test_rawControlCharacterRejected() {
|
||||
expectParseError(
|
||||
"key: value\u{01}with control\n",
|
||||
.invalidControlCharacter(line: 1, column: 11),
|
||||
)
|
||||
}
|
||||
|
||||
@Test("A tab in scalar content, not indentation, is still legal")
|
||||
func test_tabInContentStillAccepted() throws {
|
||||
let value = try PureYAML.parse("key: value\twith tab\n")
|
||||
let mapping = requireMapping(value)
|
||||
#expect(mapping?["key"] == .string("value\twith tab"))
|
||||
}
|
||||
|
||||
@Test("CR and LF are still legal")
|
||||
func test_crAndLFStillAccepted() throws {
|
||||
let value = try PureYAML.parse("key: |\n line one\n line two\n")
|
||||
let mapping = requireMapping(value)
|
||||
#expect(mapping?["key"] == .string("line one\nline two\n"))
|
||||
}
|
||||
|
||||
@Test("A control character deep inside a large document is still caught")
|
||||
func test_controlCharacterFarIntoDocumentIsCaught() {
|
||||
let padding = String(repeating: "line: value\n", count: 5000)
|
||||
expectParseError(
|
||||
padding + "bad: value\u{02}here\n",
|
||||
.invalidControlCharacter(line: 5001, column: 11),
|
||||
)
|
||||
}
|
||||
|
||||
@Test("An earlier real syntax error is not hidden behind a later control character")
|
||||
func test_earlierSyntaxErrorNotHiddenByLaterControlCharacter() {
|
||||
do {
|
||||
_ = try PureYAML.parse("key: \"unterminated\nbad: value\u{02}here\n")
|
||||
recordIssue("expected parse to throw")
|
||||
} catch let error as PureYAML.Parsing.ParseError {
|
||||
#expect(error.diagnosticCode != "invalidControlCharacter")
|
||||
} catch {
|
||||
recordIssue("expected a ParseError, got \(error)")
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user