Skip to content

Skip leading UTF-8 BOMs in YAML, Starlark, and text templates - #1000

Open
vsolano9 wants to merge 3 commits into
carvel-dev:developfrom
vsolano9:fix/999-utf8-bom
Open

Skip leading UTF-8 BOMs in YAML, Starlark, and text templates#1000
vsolano9 wants to merge 3 commits into
carvel-dev:developfrom
vsolano9:fix/999-utf8-bom

Conversation

@vsolano9

@vsolano9 vsolano9 commented Aug 5, 2026

Copy link
Copy Markdown

Fixes #999

What

A file saved with a UTF-8 BOM was parsed with the BOM as content. In YAML it became a leading U+FEFF on the first key; in Starlark it caused a compile error; and in a text template it leaked into rendered output.

The YAML path also had a second failure: ParseBytes decides whether to prepend a document marker by matching docStartMarkerCheck against the raw input. A BOM in front of a leading --- hid that marker, so the parser prepended a second one and the file did not parse.

Where the fix lives, and why

The shared files.TrimUTF8BOM helper removes only a leading UTF-8 BOM. YAML parsing invokes it before the document-marker check. Starlark and text-template evaluation invoke it before compiling or parsing their template input.

The trim deliberately does not happen in files.File.Bytes(): that method also feeds the verbatim passthrough path, where a file force-marked type=text may be binary and must retain every byte. In EvalText, the trim therefore happens only after the plain/non-template passthrough returns.

NewDocumentSetFromBytes also trims so the bytes retained for AsSourceBytes—which back source lines shown in template errors—match the bytes parsed. The operation is idempotent. A U+FEFF later in a file remains ordinary content.

Testing

  • parser coverage compares BOM-prefixed YAML with the same input without a BOM across a plain mapping, explicit ---, leading #! comment, and multi-document stream, including the hidden-marker failure
  • end-to-end template coverage exercises the reported YAML case and confirms a mid-file BOM survives
  • follow-through coverage confirms BOM-prefixed .star libraries and text templates behave exactly like their no-BOM equivalents

The new cases were confirmed failing before their respective implementation changes.

Fresh local validation on 18 Aug 2026:

  • go test ./... -count=1 — all packages pass, including test/e2e and test/filetests (with a locally built versioned ytt binary as required by the e2e harness)
  • focused BOM tests and the affected package suites pass
  • git diff --check passes; changed Go files are gofmt clean

The branch remains mergeable with current develop (a3e1f7b).

AI assistance

This patch was prepared with AI assistance. The resulting diff and test output were reviewed locally before submission.

A file saved with a UTF-8 BOM was parsed with the BOM as content, so the
first key gained a leading U+FEFF and rendered as a quoted, escaped key:
`test: #@ None` came out as `"\x74\x65\x73\x74": null`.

The BOM is a stream encoding marker rather than content, so it is now
trimmed in Parser.ParseBytes. That is ahead of the document marker check,
which matters on its own: a BOM in front of a leading `---` hid the
marker, so the parser prepended a second one and the file failed to parse
with "mapping values are not allowed in this context".

Trimming in the parser covers every YAML entry point, including data
values files, schema and `ytt fmt`. NewDocumentSetFromBytes trims as well
so the bytes it retains for AsSourceBytes, which back the source lines
shown in template errors, are the bytes that were actually parsed.

Only a leading BOM is treated as a marker; one appearing later in a file
is ordinary content and is left alone.

Fixes carvel-dev#999

Signed-off-by: Victor Solano <victor.solanonunez@gmail.com>
@carvel-bot carvel-bot added this to Carvel Aug 5, 2026
@joaopapereira

Copy link
Copy Markdown
Member

If you do not mind extending the pr I would prefer

Signed-off-by: Victor Solano <victor.solanonunez@gmail.com>
@vsolano9 vsolano9 changed the title Skip a leading UTF-8 BOM when parsing YAML Skip leading UTF-8 BOMs in YAML, Starlark, and text templates Aug 18, 2026
@vsolano9

Copy link
Copy Markdown
Author

Extended in 9228926. Leading UTF-8 BOMs are now handled for .star libraries and text templates as well as YAML, while verbatim type=text passthrough remains byte-for-byte unchanged. The new cases were red before the implementation and the full go test ./... -count=1 suite passes, including e2e/filetests.

The lint job fails on two lines in utf8_bom_test.go: revive's line-length-limit
counts 96 and 90 against a limit of 80. Hoisting the data.yml source into a named
constant and splitting the two MustNewFileFromSource calls brings both under the
limit without changing what the test does.

golangci-lint v2.12.2 (the pinned CI version) now reports only the four
pre-existing unhandled-error findings in pkg/yamlfmt and pkg/yamlmeta, neither of
which this PR touches; CI's new-from-rev filter already excludes them. go build
./... and go test ./... are green, e2e included.

Signed-off-by: Victor Solano <victor.solanonunez@gmail.com>
@vsolano9

Copy link
Copy Markdown
Author

Fixed the lint failure at 28903f0: revive's line-length-limit flagged two lines in utf8_bom_test.go at 96 and 90 characters against the limit of 80. Hoisted the data.yml source into a named variable and split the two MustNewFileFromSource calls; the test does the same thing.

Locally, golangci-lint v2.12.2 — the pinned CI version — now reports only the four pre-existing unhandled-error findings in pkg/yamlfmt and pkg/yamlmeta, neither of which this PR touches, and which new-from-rev already excludes. go build ./... and go test ./... are green, e2e included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Files with UTF-8 BOM produce incorrect output

3 participants