diff --git a/changelog/fragments/1740485771-ast-null-checks.yaml b/changelog/fragments/1740485771-ast-null-checks.yaml new file mode 100644 index 00000000000..4c9d752a437 --- /dev/null +++ b/changelog/fragments/1740485771-ast-null-checks.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: Add missing null checks to AST methods + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: elastic-agent + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +#pr: https://github.com/owner/repo/1234 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +issue: https://github.com/elastic/elastic-agent/issues/6999 diff --git a/internal/pkg/agent/transpiler/ast.go b/internal/pkg/agent/transpiler/ast.go index bacf1be208c..bb9a2e03866 100644 --- a/internal/pkg/agent/transpiler/ast.go +++ b/internal/pkg/agent/transpiler/ast.go @@ -105,6 +105,9 @@ func NewDictWithProcessors(nodes []Node, processors Processors) *Dict { // Find takes a string which is a key and try to find the elements in the associated K/V. func (d *Dict) Find(key string) (Node, bool) { for _, i := range d.value { + if i == nil { + continue + } if i.(*Key).name == key { return i, true } @@ -119,9 +122,12 @@ func (d *Dict) Insert(node Node) { func (d *Dict) String() string { var sb strings.Builder - for i := 0; i < len(d.value); i++ { + for i, node := range d.value { + if node == nil { + continue + } sb.WriteString("{") - sb.WriteString(d.value[i].String()) + sb.WriteString(node.String()) sb.WriteString("}") if i < len(d.value)-1 { sb.WriteString(",") @@ -166,6 +172,9 @@ func (d *Dict) ShallowClone() Node { func (d *Dict) Hash() []byte { h := sha256.New() for _, v := range d.value { + if v == nil { + continue + } h.Write(v.Hash()) } return h.Sum(nil) @@ -174,6 +183,9 @@ func (d *Dict) Hash() []byte { // Hash64With recursively computes the given hash for the Node and its children func (d *Dict) Hash64With(h *xxhash.Digest) error { for _, v := range d.value { + if v == nil { + continue + } if err := v.Hash64With(h); err != nil { return err } @@ -184,6 +196,9 @@ func (d *Dict) Hash64With(h *xxhash.Digest) error { // Vars returns a list of all variables referenced in the dictionary. func (d *Dict) Vars(vars []string, defaultProvider string) []string { for _, v := range d.value { + if v == nil { + continue + } k := v.(*Key) vars = k.Vars(vars, defaultProvider) } @@ -194,6 +209,9 @@ func (d *Dict) Vars(vars []string, defaultProvider string) []string { func (d *Dict) Apply(vars *Vars) (Node, error) { nodes := make([]Node, 0, len(d.value)) for _, v := range d.value { + if v == nil { + continue + } k := v.(*Key) n, err := k.Apply(vars) if err != nil { @@ -222,6 +240,9 @@ func (d *Dict) Processors() Processors { return d.processors } for _, v := range d.value { + if v == nil { + continue + } if p := v.Processors(); p != nil { return p } @@ -387,8 +408,11 @@ func NewListWithProcessors(nodes []Node, processors Processors) *List { func (l *List) String() string { var sb strings.Builder sb.WriteString("[") - for i := 0; i < len(l.value); i++ { - sb.WriteString(l.value[i].String()) + for i, v := range l.value { + if v == nil { + continue + } + sb.WriteString(v.String()) if i < len(l.value)-1 { sb.WriteString(",") } @@ -401,6 +425,9 @@ func (l *List) String() string { func (l *List) Hash() []byte { h := sha256.New() for _, v := range l.value { + if v == nil { + continue + } h.Write(v.Hash()) } @@ -410,6 +437,9 @@ func (l *List) Hash() []byte { // Hash64With recursively computes the given hash for the Node and its children func (l *List) Hash64With(h *xxhash.Digest) error { for _, v := range l.value { + if v == nil { + continue + } if err := v.Hash64With(h); err != nil { return err } @@ -465,6 +495,9 @@ func (l *List) ShallowClone() Node { // Vars returns a list of all variables referenced in the list. func (l *List) Vars(vars []string, defaultProvider string) []string { for _, v := range l.value { + if v == nil { + continue + } vars = v.Vars(vars, defaultProvider) } return vars @@ -474,6 +507,9 @@ func (l *List) Vars(vars []string, defaultProvider string) []string { func (l *List) Apply(vars *Vars) (Node, error) { nodes := make([]Node, 0, len(l.value)) for _, v := range l.value { + if v == nil { + continue + } n, err := v.Apply(vars) if err != nil { return nil, err @@ -492,6 +528,9 @@ func (l *List) Processors() Processors { return l.processors } for _, v := range l.value { + if v == nil { + continue + } if p := v.Processors(); p != nil { return p } diff --git a/internal/pkg/agent/transpiler/ast_test.go b/internal/pkg/agent/transpiler/ast_test.go index 3ea626c8624..624df1f6232 100644 --- a/internal/pkg/agent/transpiler/ast_test.go +++ b/internal/pkg/agent/transpiler/ast_test.go @@ -9,6 +9,8 @@ import ( "reflect" "testing" + "github.com/cespare/xxhash/v2" + "github.com/elastic/elastic-agent-libs/mapstr" "github.com/elastic/elastic-agent/internal/pkg/eql" @@ -1206,6 +1208,49 @@ func TestCondition(t *testing.T) { assert.Nil(t, input2.condition) } +// check that all the methods handle nil values correctly +func TestNullValues(t *testing.T) { + cfgMap := map[string]any{ + "inputs": map[string]any{ + "dict": map[string]any{ + "key": nil, + }, + "list": []any{nil}, + }, + } + ast, err := NewAST(cfgMap) + require.NoError(t, err) + inputs, ok := Lookup(ast, "inputs") + require.True(t, ok) + + assert.NotEmpty(t, inputs.String()) + + node, ok := inputs.Find("dict") + assert.True(t, ok) + assert.NotNil(t, node) + + assert.NotNil(t, inputs.Value()) + + assert.NotNil(t, inputs.Clone()) + + assert.NotNil(t, inputs.ShallowClone()) + + assert.NotEmpty(t, inputs.Hash()) + + h := xxhash.New() + err = inputs.Hash64With(h) + assert.NoError(t, err) + assert.NotEmpty(t, h.Sum64()) + + assert.Empty(t, inputs.Vars([]string{}, "default")) + + newNode, err := inputs.Apply(nil) + assert.NoError(t, err) + assert.NotNil(t, newNode) + + assert.Empty(t, inputs.Processors()) +} + func mustMakeVars(mapping map[string]interface{}) *Vars { v, err := NewVars("", mapping, nil, "") if err != nil {