Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
- Add `--stdin-filepath` flag to `buf format`, which reads a single `.proto` file from
stdin and writes the formatted result to stdout. The path is not read from disk, and is
only used to report parse errors and diffs.
- Fix configuration files silently accepting unquoted values that start with `!`, such as
`ignore: [!foo/bar.proto]`. Previously they parsed as empty strings, now they rejected
with an error.
- Fix lint comment ignores on proto2 `group` fields being ignored.
- Improve the `buf curl` error message for methods that accept a single request message.
- Deduplicate remote input fetches within a single command invocation, so that multiple
Expand Down
39 changes: 39 additions & 0 deletions private/pkg/encoding/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ func UnmarshalYAMLStrict(data []byte, v any) error {
if len(data) == 0 {
return nil
}
if err := validateNoLocalYAMLTags(data); err != nil {
return err
}
yamlDecoder := NewYAMLDecoderStrict(bytes.NewReader(data))
return updateYAMLTypeError(yamlDecoder.Decode(v))
}
Expand Down Expand Up @@ -191,6 +194,42 @@ func InterfaceSliceOrStringToStringSlice(in any) ([]string, error) {

// *** PRIVATE ***

// validateNoLocalYAMLTags returns an error if the YAML data contains a local
// tag, that is a tag of the form "!name" as opposed to a standard "!!name" tag.
// We never register custom tags that would be resolved with "!" so there is
// no valid use for a value starting with a single exclamation mark. If we don't
// detect and reject explicitly, they instead cause the tag to be trimmed from
// the value, usually leaving an empty value behind that we use as the config.
func validateNoLocalYAMLTags(data []byte) error {
var node yaml.Node
if err := yaml.Unmarshal(data, &node); err != nil {
return err
}
return walkYAMLNode(&node, func(node *yaml.Node) error {
if len(node.Tag) >= 2 && node.Tag[0] == '!' && node.Tag[1] != '!' {
return fmt.Errorf(
"yaml: line %d: unexpected tag %q: values that start with %q must be quoted",
node.Line,
node.Tag,
"!",
)
}
return nil
})
}

func walkYAMLNode(node *yaml.Node, f func(*yaml.Node) error) error {
if err := f(node); err != nil {
return err
}
for _, child := range node.Content {
if err := walkYAMLNode(child, f); err != nil {
return err
}
}
return nil
}

func updateYAMLTypeError(err error) error {
if err == nil {
return nil
Expand Down
31 changes: 31 additions & 0 deletions private/pkg/encoding/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,34 @@ func testInterfaceSliceOrStringToCommaSepString(t *testing.T, in any, expected s
require.NoError(t, err)
require.Equal(t, expected, v)
}

func TestUnmarshalYAMLStrictRejectsLocalTags(t *testing.T) {
t.Parallel()
type external struct {
Ignore []string `yaml:"ignore"`
}
// An unquoted value that starts with "!" is a YAML local tag with an
// empty value, which the decoder would otherwise silently drop.
var v external
err := UnmarshalYAMLStrict([]byte("ignore:\n - foo\n - !foo/bar.proto\n"), &v)
require.Error(t, err)
require.Contains(t, err.Error(), `line 3: unexpected tag "!foo/bar.proto"`)

v = external{}
err = UnmarshalYAMLStrict([]byte("ignore:\n - !local bar\n"), &v)
require.Error(t, err)
require.Contains(t, err.Error(), `unexpected tag "!local"`)

// Standard tags, anchors, aliases, and quoted values that start with "!"
// are all fine.
v = external{}
err = UnmarshalYAMLStrict([]byte("ignore:\n - !!str 123\n - &a foo\n - *a\n - \"!foo/bar.proto\"\n"), &v)
require.NoError(t, err)
require.Equal(t, []string{"123", "foo", "foo", "!foo/bar.proto"}, v.Ignore)

// Non-strict unmarshalling is unchanged.
v = external{}
err = UnmarshalYAMLNonStrict([]byte("ignore:\n - !foo/bar.proto\n"), &v)
require.NoError(t, err)
require.Equal(t, []string{""}, v.Ignore)
}
Loading