Repository navigation
Marshal emits a literal block scalar without an indentation indicator for strings starting with newline(s) followed by spaces; output can't be unmarshaled #976
Description
Describe the bug
Location (goccy/go-yaml @ default branch edee2f9):
token/token.go:717-731—token.LiteralBlockHeader, used byStringNode.Stringencode.go:567-569—UseLiteralStyleIfMultilinepath
// LiteralBlockHeader detect literal block scalar header
func LiteralBlockHeader(value string) string {
lbc := DetectLineBreakCharacter(value)
switch {
case !strings.Contains(value, lbc):
return ""
case strings.HasSuffix(value, fmt.Sprintf("%s%s", lbc, lbc)):
return "|+"
case strings.HasSuffix(value, lbc):
return "|"
default:
return "|-"
}
}Problem
Marshaling a string that begins with one or more line breaks followed by an indented line, e.g. "\n a\nb", produces a |- block scalar with no indentation indicator, because LiteralBlockHeader only chooses between |, |+ and |-. When reading it back, the content indentation is auto-detected from the first non-empty line ( a, 4 spaces), so the next line b falls outside the scalar and Unmarshal fails. The same happens with "\n\n a\nb" and with UseLiteralStyleIfMultiline(true).
This is not the escaped-characters-in-keys problem from #417/#418: here the key is plain and the value is a normal string.
To Reproduce
package main
import (
"fmt"
"github.com/goccy/go-yaml"
)
func main() {
src := map[string]string{"str": "\n a\nb"}
out, err := yaml.Marshal(src)
fmt.Printf("%q %v\n", out, err)
var dst map[string]string
fmt.Println(yaml.Unmarshal(out, &dst))
}Emitted YAML (line 2 contains two spaces):
str: |-
a
bUnmarshal error (verbatim):
[4:3] value is not allowed in this context. map key-value is pre-defined
1 | str: |-
2 |
3 | a
> 4 | b
^
For "\n\n a\nb" the error is the same at [5:3], and yaml.MarshalWithOptions(src, yaml.UseLiteralStyleIfMultiline(true)) gives the same output and error as above.
Expected behavior
Unmarshal(Marshal(v)) returns the original string. YAML 1.2.2 8.1.1.1 Block Indentation Indicator: without an indicator, "the content indentation level is equal to the number of leading spaces on the first non-empty line", and "a YAML processor should only emit an explicit indentation indicator for cases where detection will fail", which is the case here. Emitting e.g.
str: |2-
a
b(production [163] c-indentation-indicator) would round-trip. Quoting the value would also work.
Version Variables
- Go version: go1.24.13 linux/amd64
- go-yaml's Version:
masteratedee2f9(v1.19.2-5-gedee2f9)
Additional context
Already fixed in nodeca/js-yaml
js-yaml is another YAML implementation of the same spec; the YAML 1.2.2 rule above is the primary basis, and js-yaml is precedent for the same case. PR: nodeca/js-yaml#404 — "Check for leading newlines when determining if block indentation indicator is needed" (merged 2018-03-16T03:37:54Z, merge commit bab69b5df4e). The dumper used to add an indentation indicator only when the string started with a space; it now adds one whenever the content starts with a space after optional leading newlines (/^\n* /). Its regression test uses the same inputs as above ('\n a\nb', '\n\n a\nb').
How js-yaml fixed it:
--- a/lib/js-yaml/dumper.js
+++ b/lib/js-yaml/dumper.js
@@ -234,6 +234,12 @@ function isPlainSafeFirst(c) {
&& c !== CHAR_GRAVE_ACCENT;
}
+// Determines whether block indentation indicator is required.
+function needIndentIndicator(string) {
+ var leadingSpaceRe = /^\n* /;
+ return leadingSpaceRe.test(string);
+}
+
var STYLE_PLAIN = 1,
STYLE_SINGLE = 2,
STYLE_LITERAL = 3,
@@ -365,7 +371,7 @@ function writeScalar(state, string, level, iskey) {
// Pre-conditions: string is valid for a block scalar, 1 <= indentPerLevel <= 9.
function blockHeader(string, indentPerLevel) {
- var indentIndicator = (string[0] === ' ') ? String(indentPerLevel) : '';
+ var indentIndicator = needIndentIndicator(string) ? String(indentPerLevel) : '';
// note the special case: the string '\n' counts as a "trailing" empty line.
var clip = string[string.length - 1] === '\n';
--- a/test/issues/0403.js
+++ b/test/issues/0403.js
@@ -0,0 +1,22 @@
+'use strict';
+
+
+var assert = require('assert');
+var yaml = require('../../');
+
+
+test('should properly dump leading newlines and spaces', function () {
+ var dump, src;
+
+ src = { str: '\n a\nb' };
+ dump = yaml.dump(src);
+ assert.deepEqual(yaml.safeLoad(dump), src);
+
+ src = { str: '\n\n a\nb' };
+ dump = yaml.dump(src);
+ assert.deepEqual(yaml.safeLoad(dump), src);
+
+ src = { str: '\n a\nb' };
+ dump = yaml.dump(src, { indent: 10 });
+ assert.deepEqual(yaml.safeLoad(dump), src);
+});Suggested fix
Related: open PR #792 adds a leading \n to token.IsNeedQuoted, which would quote this value on the default path, but with UseLiteralStyleIfMultiline(true) Encoder.isNeedQuoted returns false for multi-line strings before calling token.IsNeedQuoted, so that path would still emit |- without an indicator; PR #968 (lone "\n" chomping) is a different case. Open #974 / PR #975 fix the decoding side of explicit indentation indicators (trailing empty lines at EOF were rejected): a different bug, but related, since once the encoder emits an explicit indicator for this case, a value that also ends in blank lines would rely on #975 to round-trip.
Like js-yaml's needIndentIndicator, the patch below emits an explicit indentation indicator (the encoder's indent width, e.g. |2-) whenever the value starts with a space after optional leading line breaks; it is applied in StringNode's header construction, so both the default path and UseLiteralStyleIfMultiline(true) are covered:
diff --git a/ast/ast.go b/ast/ast.go
--- a/ast/ast.go
+++ b/ast/ast.go
@@ -807,6 +807,22 @@ func escapeSingleQuote(s string) string {
return sb.String()
}
+// literalBlockHeader returns the literal block scalar header for the value.
+// An explicit indentation indicator is added when the content starts with a space
+// after optional leading line breaks, because the indentation cannot be detected
+// from the first non-empty line in that case.
+func (n *StringNode) literalBlockHeader(lbc string) string {
+ header := token.LiteralBlockHeader(n.Value)
+ indentNum := n.Token.Position.IndentNum
+ if indentNum < 1 || indentNum > 9 {
+ return header
+ }
+ if !strings.HasPrefix(strings.TrimLeft(n.Value, lbc), " ") {
+ return header
+ }
+ return header[:1] + strconv.Itoa(indentNum) + header[1:]
+}
+
// String string value to text with quote or literal header if required
func (n *StringNode) String() string {
switch n.Token.Type {
@@ -828,7 +844,7 @@ func (n *StringNode) String() string {
if strings.Contains(n.Value, lbc) {
// This block assumes that the line breaks in this inside scalar content and the Outside scalar content are the same.
// It works mostly, but inconsistencies occur if line break characters are mixed.
- header := token.LiteralBlockHeader(n.Value)
+ header := n.literalBlockHeader(lbc)
space := strings.Repeat(" ", n.Token.Position.Column-1)
indent := strings.Repeat(" ", n.Token.Position.IndentNum)
values := []string{}
@@ -860,7 +876,7 @@ func (n *StringNode) stringWithoutComment() string {
if strings.Contains(n.Value, lbc) {
// This block assumes that the line breaks in this inside scalar content and the Outside scalar content are the same.
// It works mostly, but inconsistencies occur if line break characters are mixed.
- header := token.LiteralBlockHeader(n.Value)
+ header := n.literalBlockHeader(lbc)
space := strings.Repeat(" ", n.Token.Position.Column-1)
indent := strings.Repeat(" ", n.Token.Position.IndentNum)
values := []string{}Verified on edee2f9 (go1.24.13 linux/amd64): a regression test that round-trips the three cases above ("\n a\nb", "\n\n a\nb", and "\n a\nb" with UseLiteralStyleIfMultiline(true)) fails without this patch and passes with it; no other test results change (go test ./...).
I can submit a PR for this.
Activity
fix(token): emit literal indentation indicator for /^\n* / values