close
Skip to content

Yaml: fix decoding empty stream everywhere#29690

Merged
andig merged 3 commits into
masterfrom
fix/yaml
May 6, 2026
Merged

Yaml: fix decoding empty stream everywhere#29690
andig merged 3 commits into
masterfrom
fix/yaml

Conversation

@andig
Copy link
Copy Markdown
Member

@andig andig commented May 6, 2026

Fix #29672 (for real)

@andig andig added the bug Something isn't working label May 6, 2026
@andig andig changed the title Yaml: fix decoding empty stream everwhere Yaml: fix decoding empty stream everywhere May 6, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In util/yaml/yaml.go, Unmarshal currently calls goyaml.Unmarshal(b, &res), which will pass a pointer to the interface instead of the underlying value; this should likely be goyaml.Unmarshal(b, res) so callers can pass their destination directly.
  • Consider extending the util/yaml package to expose Encoder/Decoder helpers (e.g., NewEncoder/NewDecoder) so that call sites like settings.SetYaml can consistently use the wrapper instead of mixing direct goyaml usage with the custom Unmarshal behavior.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In util/yaml/yaml.go, Unmarshal currently calls goyaml.Unmarshal(b, &res), which will pass a pointer to the interface instead of the underlying value; this should likely be goyaml.Unmarshal(b, res) so callers can pass their destination directly.
- Consider extending the util/yaml package to expose Encoder/Decoder helpers (e.g., NewEncoder/NewDecoder) so that call sites like settings.SetYaml can consistently use the wrapper instead of mixing direct goyaml usage with the custom Unmarshal behavior.

## Individual Comments

### Comment 1
<location path="util/yaml/yaml.go" line_range="14-15" />
<code_context>
+}
+
+// Unmarshal invokes unmarshaler, ignoring empty document errors
+func Unmarshal(b []byte, res any) error {
+	err := goyaml.Unmarshal(b, &res)
+	if err != nil && strings.Contains(err.Error(), "no documents in stream") {
+		err = nil
</code_context>
<issue_to_address>
**issue (bug_risk):** Unmarshal is unmarshalling into a copy of `res` instead of the caller’s value

Here `res` should be passed directly to `goyaml.Unmarshal`. Because `res` is already a pointer to the target value, using `&res` gives `Unmarshal` a `*interface{}` (or pointer-to-`any`), so it only populates that temporary rather than the caller’s value. This means the caller observes no changes. Use `goyaml.Unmarshal(b, res)` instead.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread util/yaml/yaml.go Outdated
@andig andig enabled auto-merge (squash) May 6, 2026 07:57
@andig andig disabled auto-merge May 6, 2026 07:58
Comment thread util/yaml/yaml.go Outdated
@andig andig merged commit 42b5cb2 into master May 6, 2026
7 checks passed
@andig andig deleted the fix/yaml branch May 6, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

yaml construct error on empty/comment-only stream

1 participant