fix: report C0 control characters in scalar values as errors (#703) - #704
Open
wvwoo wants to merge 1 commit into
Open
fix: report C0 control characters in scalar values as errors (#703)#704wvwoo wants to merge 1 commit into
wvwoo wants to merge 1 commit into
Conversation
) Literal C0 control characters (other than the permitted tab, line feed and carriage return) are excluded from the YAML `c-printable` production, but the composer accepted them silently in plain, quoted and block scalars. Add a `checkPrintableChars` helper that scans the raw scalar source and emits a `CONTROL_CHAR` error for each disallowed C0 control, wired into `resolveFlowScalar` and `resolveBlockScalar`. The check runs on the raw source rather than the resolved value, so escape sequences such as `\t` or `\x07` in double-quoted scalars remain valid.
eemeli
reviewed
Jul 12, 2026
| ): void { | ||
| controlCharRe.lastIndex = 0 | ||
| let match: RegExpExecArray | null | ||
| while ((match = controlCharRe.exec(source)) !== null) { |
Owner
There was a problem hiding this comment.
Why this, rather than source.match(controlCharRe)?
And hello, human person, this is a question for you, not your LLM.
Comment on lines
+3
to
+19
| /** | ||
| * C0 control characters that are excluded from the YAML 1.2 `c-printable` | ||
| * production, i.e. every C0 control other than the allowed `\t` (#x9), | ||
| * `\n` (#xA) and `\r` (#xD). | ||
| * | ||
| * See https://yaml.org/spec/1.2.2/#rule-c-printable | ||
| */ | ||
| const controlCharRe = /[\x00-\x08\x0b\x0c\x0e-\x1f]/g | ||
|
|
||
| /** | ||
| * Reports a `CONTROL_CHAR` error for each C0 control character found in | ||
| * `source`, positioned at `offset + index`. | ||
| * | ||
| * The check is applied to the raw scalar source rather than its resolved | ||
| * value, so escape sequences in double-quoted scalars (e.g. `\t` or `\x07`) | ||
| * are not affected. | ||
| */ |
Owner
There was a problem hiding this comment.
Are these comments really adding value?
Again, this is a question for the human here.
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
2 participants
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.
Summary
Literal C0 control characters (other than the permitted
\t,\nand\r) areexcluded from the YAML 1.2
c-printableproduction, but the composer acceptedthem silently in plain, quoted and block scalars.
Fixes #703.
Changes
src/compose/util-check-printable.ts: newcheckPrintableCharshelper — scansthe raw scalar source and emits a
CONTROL_CHARerror for each disallowed C0control. It runs on the raw source rather than the resolved value, so escape
sequences such as
\tor\x07in double-quoted scalars remain valid.src/compose/resolve-flow-scalar.ts/src/compose/resolve-block-scalar.ts:wire in the check for plain/single/double-quoted and block scalars.
src/errors.ts: add theCONTROL_CHARerror code.tests/doc/errors.ts: new tests covering control chars in each scalar style,plus regression guards for escaped controls and permitted whitespace (tab/NEL).
Scope note
Scoped to C0 controls only (
\x00–\x08,\x0b,\x0c,\x0e–\x1f), matching theissue title. DEL (
\x7f) and C1 controls are intentionally left accepted so theofficial JSON test suite (
y_string_with_del_character,y_string_unescaped_char_delete) keeps passing — JSON permits unescaped DEL andthis parser is deliberately JSON-compatible. Happy to extend further if you'd
rather also revisit those JSON-compat cases.
Testing
Full suite passes:
npx vitest run— 3386 tests passed, 11 skipped, 0 failed,including the official
yaml-test-suiteandJSONTestSuitesubmodules.tsc --noEmit,eslint, andprettier --checkare all clean. The 5 new"should error" tests were confirmed to fail on the pre-fix code and pass with
the fix; the 2 regression-guard tests pass throughout.
LLM use disclosure
Per this project's CONTRIBUTING.md, disclosing as required: this PR was produced
by an autonomous coding agent (Claude). A human reviewed the diff and test results
before submission.