diff --git a/kyaml/internal/forked/github.com/go-yaml/yaml/.github/workflows/semgrep.yaml b/kyaml/internal/forked/github.com/go-yaml/yaml/.github/workflows/semgrep.yaml deleted file mode 100644 index 200c00a5a..000000000 --- a/kyaml/internal/forked/github.com/go-yaml/yaml/.github/workflows/semgrep.yaml +++ /dev/null @@ -1,16 +0,0 @@ ---- -name: Semgrep -on: [push, pull_request] -jobs: - semgrep: - name: semgrep - runs-on: ubuntu-20.04 - steps: - - uses: actions/checkout@v2 - with: - fetch-depth: 0 - - name: Semgrep - id: semgrep - uses: returntocorp/semgrep-action@v1 - with: - config: p/dgryski.semgrep-go diff --git a/kyaml/internal/forked/github.com/go-yaml/yaml/decode.go b/kyaml/internal/forked/github.com/go-yaml/yaml/decode.go index df36e3a30..0173b6982 100644 --- a/kyaml/internal/forked/github.com/go-yaml/yaml/decode.go +++ b/kyaml/internal/forked/github.com/go-yaml/yaml/decode.go @@ -100,7 +100,10 @@ func (p *parser) peek() yaml_event_type_t { if p.event.typ != yaml_NO_EVENT { return p.event.typ } - if !yaml_parser_parse(&p.parser, &p.event) { + // It's curious choice from the underlying API to generally return a + // positive result on success, but on this case return true in an error + // scenario. This was the source of bugs in the past (issue #666). + if !yaml_parser_parse(&p.parser, &p.event) || p.parser.error != yaml_NO_ERROR { p.fail() } return p.event.typ @@ -320,6 +323,8 @@ type decoder struct { decodeCount int aliasCount int aliasDepth int + + mergedFields map[interface{}]bool } var ( @@ -808,6 +813,11 @@ func (d *decoder) mapping(n *Node, out reflect.Value) (good bool) { } } + mergedFields := d.mergedFields + d.mergedFields = nil + + var mergeNode *Node + mapIsNew := false if out.IsNil() { out.Set(reflect.MakeMap(outt)) @@ -815,11 +825,18 @@ func (d *decoder) mapping(n *Node, out reflect.Value) (good bool) { } for i := 0; i < l; i += 2 { if isMerge(n.Content[i]) { - d.merge(n.Content[i+1], out) + mergeNode = n.Content[i+1] continue } k := reflect.New(kt).Elem() if d.unmarshal(n.Content[i], k) { + if mergedFields != nil { + ki := k.Interface() + if mergedFields[ki] { + continue + } + mergedFields[ki] = true + } kkind := k.Kind() if kkind == reflect.Interface { kkind = k.Elem().Kind() @@ -833,6 +850,12 @@ func (d *decoder) mapping(n *Node, out reflect.Value) (good bool) { } } } + + d.mergedFields = mergedFields + if mergeNode != nil { + d.merge(n, mergeNode, out) + } + d.stringMapType = stringMapType d.generalMapType = generalMapType return true @@ -844,7 +867,8 @@ func isStringMap(n *Node) bool { } l := len(n.Content) for i := 0; i < l; i += 2 { - if n.Content[i].ShortTag() != strTag { + shortTag := n.Content[i].ShortTag() + if shortTag != strTag && shortTag != mergeTag { return false } } @@ -861,7 +885,6 @@ func (d *decoder) mappingStruct(n *Node, out reflect.Value) (good bool) { var elemType reflect.Type if sinfo.InlineMap != -1 { inlineMap = out.Field(sinfo.InlineMap) - inlineMap.Set(reflect.New(inlineMap.Type()).Elem()) elemType = inlineMap.Type().Elem() } @@ -870,6 +893,9 @@ func (d *decoder) mappingStruct(n *Node, out reflect.Value) (good bool) { d.prepare(n, field) } + mergedFields := d.mergedFields + d.mergedFields = nil + var mergeNode *Node var doneFields []bool if d.uniqueKeys { doneFields = make([]bool, len(sinfo.FieldsList)) @@ -879,13 +905,20 @@ func (d *decoder) mappingStruct(n *Node, out reflect.Value) (good bool) { for i := 0; i < l; i += 2 { ni := n.Content[i] if isMerge(ni) { - d.merge(n.Content[i+1], out) + mergeNode = n.Content[i+1] continue } if !d.unmarshal(ni, name) { continue } - if info, ok := sinfo.FieldsMap[name.String()]; ok { + sname := name.String() + if mergedFields != nil { + if mergedFields[sname] { + continue + } + mergedFields[sname] = true + } + if info, ok := sinfo.FieldsMap[sname]; ok { if d.uniqueKeys { if doneFields[info.Id] { d.terrors = append(d.terrors, fmt.Sprintf("line %d: field %s already set in type %s", ni.Line, name.String(), out.Type())) @@ -911,6 +944,11 @@ func (d *decoder) mappingStruct(n *Node, out reflect.Value) (good bool) { d.terrors = append(d.terrors, fmt.Sprintf("line %d: field %s not found in type %s", ni.Line, name.String(), out.Type())) } } + + d.mergedFields = mergedFields + if mergeNode != nil { + d.merge(n, mergeNode, out) + } return true } @@ -918,19 +956,29 @@ func failWantMap() { failf("map merge requires map or sequence of maps as the value") } -func (d *decoder) merge(n *Node, out reflect.Value) { - switch n.Kind { +func (d *decoder) merge(parent *Node, merge *Node, out reflect.Value) { + mergedFields := d.mergedFields + if mergedFields == nil { + d.mergedFields = make(map[interface{}]bool) + for i := 0; i < len(parent.Content); i += 2 { + k := reflect.New(ifaceType).Elem() + if d.unmarshal(parent.Content[i], k) { + d.mergedFields[k.Interface()] = true + } + } + } + + switch merge.Kind { case MappingNode: - d.unmarshal(n, out) + d.unmarshal(merge, out) case AliasNode: - if n.Alias != nil && n.Alias.Kind != MappingNode { + if merge.Alias != nil && merge.Alias.Kind != MappingNode { failWantMap() } - d.unmarshal(n, out) + d.unmarshal(merge, out) case SequenceNode: - // Step backwards as earlier nodes take precedence. - for i := len(n.Content) - 1; i >= 0; i-- { - ni := n.Content[i] + for i := 0; i < len(merge.Content); i++ { + ni := merge.Content[i] if ni.Kind == AliasNode { if ni.Alias != nil && ni.Alias.Kind != MappingNode { failWantMap() @@ -943,6 +991,8 @@ func (d *decoder) merge(n *Node, out reflect.Value) { default: failWantMap() } + + d.mergedFields = mergedFields } func isMerge(n *Node) bool { diff --git a/kyaml/internal/forked/github.com/go-yaml/yaml/decode_test.go b/kyaml/internal/forked/github.com/go-yaml/yaml/decode_test.go index 7b34fade2..bbd9d70da 100644 --- a/kyaml/internal/forked/github.com/go-yaml/yaml/decode_test.go +++ b/kyaml/internal/forked/github.com/go-yaml/yaml/decode_test.go @@ -767,7 +767,7 @@ var unmarshalTests = []struct { M{"a": 123456e1}, }, { "a: 123456E1\n", - M{"a": 123456E1}, + M{"a": 123456e1}, }, // yaml-test-suite 3GZX: Spec Example 7.1. Alias Nodes { @@ -802,7 +802,6 @@ var unmarshalTests = []struct { "c": []interface{}{"d", "e"}, }, }, - } type M map[string]interface{} @@ -948,16 +947,18 @@ var unmarshalErrorTests = []struct { {"%TAG !%79! tag:yaml.org,2002:\n---\nv: !%79!int '1'", "yaml: did not find expected whitespace"}, {"a:\n 1:\nb\n 2:", ".*could not find expected ':'"}, {"a: 1\nb: 2\nc 2\nd: 3\n", "^yaml: line 3: could not find expected ':'$"}, + {"#\n-\n{", "yaml: line 3: could not find expected ':'"}, // Issue #665 + {"0: [:!00 \xef", "yaml: incomplete UTF-8 octet sequence"}, // Issue #666 { "a: &a [00,00,00,00,00,00,00,00,00]\n" + - "b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]\n" + - "c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]\n" + - "d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]\n" + - "e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]\n" + - "f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]\n" + - "g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]\n" + - "h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]\n" + - "i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]\n", + "b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]\n" + + "c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]\n" + + "d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]\n" + + "e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]\n" + + "f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]\n" + + "g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]\n" + + "h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]\n" + + "i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]\n", "yaml: document contains excessive aliasing", }, } @@ -1391,7 +1392,7 @@ inlineSequenceMap: ` func (s *S) TestMerge(c *C) { - var want = map[interface{}]interface{}{ + var want = map[string]interface{}{ "x": 1, "y": 2, "r": 10, @@ -1436,7 +1437,103 @@ func (s *S) TestMergeStruct(c *C) { } } -var unmarshalNullTests = []struct{ input string; pristine, expected func() interface{} }{{ +var mergeTestsNested = ` +mergeouter1: &mergeouter1 + d: 40 + e: 50 + +mergeouter2: &mergeouter2 + e: 5 + f: 6 + g: 70 + +mergeinner1: &mergeinner1 + <<: *mergeouter1 + inner: + a: 1 + b: 2 + +mergeinner2: &mergeinner2 + <<: *mergeouter2 + inner: + a: -1 + b: -2 + +outer: + <<: [*mergeinner1, *mergeinner2] + f: 60 + inner: + a: 10 +` + +func (s *S) TestMergeNestedStruct(c *C) { + // Issue #818: Merging used to just unmarshal twice on the target + // value, which worked for maps as these were replaced by the new map, + // but not on struct values as these are preserved. This resulted in + // the nested data from the merged map to be mixed up with the data + // from the map being merged into. + // + // This test also prevents two potential bugs from showing up: + // + // 1) A simple implementation might just zero out the nested value + // before unmarshaling the second time, but this would clobber previous + // data that is usually respected ({C: 30} below). + // + // 2) A simple implementation might attempt to handle the key skipping + // directly by iterating over the merging map without recursion, but + // there are more complex cases that require recursion. + // + // Quick summary of the fields: + // + // - A must come from outer and not overriden + // - B must not be set as its in the ignored merge + // - C should still be set as it's preset in the value + // - D should be set from the recursive merge + // - E should be set from the first recursive merge, ignored on the second + // - F should be set in the inlined map from outer, ignored later + // - G should be set in the inlined map from the second recursive merge + // + + type Inner struct { + A, B, C int + } + type Outer struct { + D, E int + Inner Inner + Inline map[string]int `yaml:",inline"` + } + type Data struct { + Outer Outer + } + + test := Data{Outer{0, 0, Inner{C: 30}, nil}} + want := Data{Outer{40, 50, Inner{A: 10, C: 30}, map[string]int{"f": 60, "g": 70}}} + + err := yaml.Unmarshal([]byte(mergeTestsNested), &test) + c.Assert(err, IsNil) + c.Assert(test, DeepEquals, want) + + // Repeat test with a map. + + var testm map[string]interface{} + var wantm = map[string]interface {} { + "f": 60, + "inner": map[string]interface{}{ + "a": 10, + }, + "d": 40, + "e": 50, + "g": 70, + } + err = yaml.Unmarshal([]byte(mergeTestsNested), &testm) + c.Assert(err, IsNil) + c.Assert(testm["outer"], DeepEquals, wantm) +} + +var unmarshalNullTests = []struct { + input string + pristine, expected func() interface{} +}{{ "null", func() interface{} { var v interface{}; v = "v"; return &v }, func() interface{} { var v interface{}; v = nil; return &v }, @@ -1487,7 +1584,7 @@ func (s *S) TestUnmarshalNull(c *C) { func (s *S) TestUnmarshalPreservesData(c *C) { var v struct { A, B int - C int `yaml:"-"` + C int `yaml:"-"` } v.A = 42 v.C = 88 diff --git a/kyaml/internal/forked/github.com/go-yaml/yaml/parserc.go b/kyaml/internal/forked/github.com/go-yaml/yaml/parserc.go index ac66fccc0..268558a0d 100644 --- a/kyaml/internal/forked/github.com/go-yaml/yaml/parserc.go +++ b/kyaml/internal/forked/github.com/go-yaml/yaml/parserc.go @@ -687,6 +687,9 @@ func yaml_parser_parse_node(parser *yaml_parser_t, event *yaml_event_t, block, i func yaml_parser_parse_block_sequence_entry(parser *yaml_parser_t, event *yaml_event_t, first bool) bool { if first { token := peek_token(parser) + if token == nil { + return false + } parser.marks = append(parser.marks, token.start_mark) skip_token(parser) } @@ -786,7 +789,7 @@ func yaml_parser_split_stem_comment(parser *yaml_parser_t, stem_len int) { } token := peek_token(parser) - if token.typ != yaml_BLOCK_SEQUENCE_START_TOKEN && token.typ != yaml_BLOCK_MAPPING_START_TOKEN { + if token == nil || token.typ != yaml_BLOCK_SEQUENCE_START_TOKEN && token.typ != yaml_BLOCK_MAPPING_START_TOKEN { return } @@ -813,6 +816,9 @@ func yaml_parser_split_stem_comment(parser *yaml_parser_t, stem_len int) { func yaml_parser_parse_block_mapping_key(parser *yaml_parser_t, event *yaml_event_t, first bool) bool { if first { token := peek_token(parser) + if token == nil { + return false + } parser.marks = append(parser.marks, token.start_mark) skip_token(parser) } @@ -922,6 +928,9 @@ func yaml_parser_parse_block_mapping_value(parser *yaml_parser_t, event *yaml_ev func yaml_parser_parse_flow_sequence_entry(parser *yaml_parser_t, event *yaml_event_t, first bool) bool { if first { token := peek_token(parser) + if token == nil { + return false + } parser.marks = append(parser.marks, token.start_mark) skip_token(parser) }