From adcfcfa2e78718814cb4e27139aa043946a03424 Mon Sep 17 00:00:00 2001 From: Yoav Date: Sat, 3 Jul 2021 12:54:31 +0300 Subject: [PATCH] add yaml separator validation and avoid silent ignoration --- .../apimachinery/pkg/util/yaml/decoder.go | 18 +++++---- .../pkg/util/yaml/decoder_test.go | 38 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go index 612d63a6948..56f4262aca8 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder.go @@ -291,15 +291,19 @@ func (r *YAMLReader) Read() ([]byte, error) { if i := bytes.Index(line, []byte(separator)); i == 0 { // We have a potential document terminator i += sep - after := line[i:] - if len(strings.TrimRightFunc(string(after), unicode.IsSpace)) == 0 { - if buffer.Len() != 0 { - return buffer.Bytes(), nil - } - if err == io.EOF { - return nil, err + trimmed := strings.TrimSpace(string(line[i:])) + // We only allow comments and spaces following the yaml doc separator, otherwise we'll return an error + if len(trimmed) > 0 && string(trimmed[0]) != "#" { + return nil, YAMLSyntaxError{ + err: fmt.Errorf("invalid Yaml document separator: %s", trimmed), } } + if buffer.Len() != 0 { + return buffer.Bytes(), nil + } + if err == io.EOF { + return nil, err + } } if err == io.EOF { if buffer.Len() != 0 { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go index fbbba51fcc7..45e6c034ed9 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/yaml/decoder_test.go @@ -211,6 +211,40 @@ stuff: 1 } } +func TestDecodeYAMLSeparatorValidation(t *testing.T) { + s := NewYAMLToJSONDecoder(bytes.NewReader([]byte(`--- +stuff: 1 +--- # Make sure termination happen with inline comment +stuff: 2 +--- +stuff: 3 +--- Make sure uncommented content results YAMLSyntaxError + + `))) + obj := generic{} + if err := s.Decode(&obj); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fmt.Sprintf("%#v", obj) != `yaml.generic{"stuff":1}` { + t.Errorf("unexpected object: %#v", obj) + } + obj = generic{} + if err := s.Decode(&obj); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fmt.Sprintf("%#v", obj) != `yaml.generic{"stuff":2}` { + t.Errorf("unexpected object: %#v", obj) + } + obj = generic{} + err := s.Decode(&obj) + if err == nil { + t.Fatalf("expected YamlSyntaxError, got nil instead") + } + if _, ok := err.(YAMLSyntaxError); !ok { + t.Fatalf("unexpected error: %v", err) + } +} + func TestDecodeBrokenYAML(t *testing.T) { s := NewYAMLOrJSONDecoder(bytes.NewReader([]byte(`--- stuff: 1 @@ -282,6 +316,10 @@ func TestYAMLOrJSONDecoder(t *testing.T) { {"foo": "bar"}, {"baz": "biz"}, }}, + {"---\nfoo: bar\n--- # with Comment\nbaz: biz", 100, false, false, []generic{ + {"foo": "bar"}, + {"baz": "biz"}, + }}, {"foo: bar\n---\n", 100, false, false, []generic{ {"foo": "bar"}, }},