Skip to content

yaml: fix surgical Patch() for the empty-streams config round-trip - #2355

Open
firepinn wants to merge 2 commits into
AlexxIT:masterfrom
firepinn:fix-yaml-null-scalar-and-missing-path
Open

yaml: fix surgical Patch() for the empty-streams config round-trip#2355
firepinn wants to merge 2 commits into
AlexxIT:masterfrom
firepinn:fix-yaml-null-scalar-and-missing-path

Conversation

@firepinn

Copy link
Copy Markdown

Problem

go2rtc edits its YAML config in place with the surgical yaml.Patch() (used by the PATCH /api/config and streams API paths) rather than re-marshaling the whole document. Two edge cases in that patcher break the common "delete the last stream, then add one back" round-trip:

  1. Deleting a path whose parent doesn't exist returns an error. addToEnd returns a fresh errors.New("yaml: path not exist"), which PatchConfig surfaces as a failure. A caller that deletes a possibly-absent key (e.g. removing a temporary stream) can't distinguish this from a real error.

  2. Adding a child under an explicit null scalar fails to parse. yaml.Marshal of an emptied map emits the explicit token key: null. Patch() then pastes the new child mapping after that scalar, producing key: null\n child: ..., which is invalid YAML (mapping values are not allowed in this context). The patch is rejected (the API returns HTTP 400) and the key can't get its first child back without resetting the file.

Together these mean: delete every stream → the streams map serializes to streams: null → adding a stream back fails.

Fix

  • Export the sentinel yaml.ErrPathNotExist and have PatchConfig treat a delete (value == nil) of a non-existent path as a no-op via errors.Is, instead of returning an error.
  • When the parent key carries an explicit null scalar on its own line, strip that scalar so the key becomes a bare mapping parent before appending the child. A bare/implicit-null value (key:) is unaffected.

Tests

Added unit tests in pkg/yaml/yaml_test.go:

  • deleting a path with an absent parent returns ErrPathNotExist;
  • adding a child under key: null, both alongside a sibling key and on its own.

go test ./pkg/yaml/ passes.

firepinn added 2 commits July 15, 2026 11:55
addToEnd returned a fresh unnamed error value when asked to delete a path
whose parent does not exist, so callers could not distinguish it from a
real failure. Export it as the sentinel ErrPathNotExist and have
PatchConfig treat a delete of a non-existent path (value == nil) as a
no-op via errors.Is, instead of surfacing an error.

Add a yaml test asserting that deleting a path with an absent parent
returns ErrPathNotExist.
yaml.Marshal of an empty map emits the explicit token "key: null". The
surgical Patch() then cannot add the first child back: it pastes the
child mapping after the null scalar, yielding "key: null\n  child: ...",
which fails to parse ("mapping values are not allowed in this context").

Strip the explicit null token so the key becomes a bare mapping parent
before appending the child. A bare/implicit-null value ("key:") carries
no token and keeps the previous behaviour.

Add tests covering an explicit-null parent both alongside a sibling key
and on its own.
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.

1 participant