Skip to content

fix: quote flow scalars where a colon precedes a flow indicator - #773

Merged
puzrin merged 1 commit into
nodeca:masterfrom
spokodev:fix/flow-colon-quoting
Jul 9, 2026
Merged

puzrin merged 1 commit into
nodeca:masterfrom
spokodev:fix/flow-colon-quoting

Conversation

@spokodev

@spokodev spokodev commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Problem

Flow-style dump produces output the library cannot re-parse when a plain scalar contains a : immediately followed by a flow indicator ({ } [ ] ,):

const yaml = require('js-yaml')
yaml.dump([':{'], { flowLevel: 0 })       // "[:{]\n"
yaml.load(yaml.dump([':{'], { flowLevel: 0 }))
// YAMLException: missed comma between flow collection entries (1:4)

This breaks load(dump(x)) for values such as ':{', ':[', ':,', ':}', ':]', and 'x:{', in both flow sequences and flow mappings. Block style is unaffected — there the scalar stays plain and round-trips fine, which is correct.

Root cause

isPlainSafe in src/ast/presenter.ts decides whether a character may appear unescaped in a plain scalar. Its last clause implements the spec's ns-plain-char rule for a : followed by content:

[130] ns-plain-char(c) ::= ... | ( ":" /* Followed by an ns-plain-safe(c) */ )

The clause was (prev === CHAR_COLON && cIsNsChar), i.e. it treated any ns-char after a colon as safe. But ns-plain-safe(c) is context-dependent:

  • [128] ns-plain-safe-out ::= ns-char (block context)
  • [129] ns-plain-safe-in ::= ns-char - c-flow-indicator (flow context)

In flow context the following character must additionally not be a flow indicator. Because the clause skipped that exclusion, { } [ ] , after a : were wrongly accepted, so the scalar was emitted plain and the {/[/, reopened or closed the surrounding flow collection.

Fix

Add the c-flow-indicator exclusion to the colon clause when not in block context, matching ns-plain-safe-in. The #-after-colon case (:#) and normal :x stay plain; flow indicators after a colon now force quoting.

Verification

Added flowLevel — quotes a colon followed by a flow indicator to test/core/units/dump-options.test.mjs, asserting load(dump(value, { flowLevel: 0 })) === value for the affected strings in flow sequences and mappings.

  • Before: the new test throws missed comma between flow collection entries.
  • After: it passes; full suite green (npm test, 1599 passing, 0 failing). No over-quoting — :a, a:b, :#, http://x remain plain in flow.

dist/ is gitignored, so no build artifacts are committed.

@puzrin
puzrin merged commit bd7ebb2 into nodeca:master Jul 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants