From eff9c5a13d00d8ec175e5d59309b0e84cee5a8b2 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 19 Mar 2019 16:08:03 +0100 Subject: [PATCH 1/6] process objects and arrays only --- .../processors/actions/decode_json_fields.go | 4 ++ .../actions/decode_json_fields_test.go | 66 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/libbeat/processors/actions/decode_json_fields.go b/libbeat/processors/actions/decode_json_fields.go index d8832858dac..da5b0a7ca65 100644 --- a/libbeat/processors/actions/decode_json_fields.go +++ b/libbeat/processors/actions/decode_json_fields.go @@ -146,6 +146,10 @@ func unmarshal(maxDepth int, text string, fields *interface{}, processArray bool return v, false } + if !strings.HasPrefix(str, "[") && !strings.HasPrefix(str, "{") { + return str, false + } + var tmp interface{} err := unmarshal(maxDepth, str, &tmp, processArray) if err != nil { diff --git a/libbeat/processors/actions/decode_json_fields_test.go b/libbeat/processors/actions/decode_json_fields_test.go index 15afb944988..7a5c7a2ac78 100644 --- a/libbeat/processors/actions/decode_json_fields_test.go +++ b/libbeat/processors/actions/decode_json_fields_test.go @@ -199,6 +199,72 @@ func TestTargetRootOption(t *testing.T) { assert.Equal(t, expected.String(), actual.String()) } +func TestNotJsonObjectOrArrayDepthOne(t *testing.T) { + input := common.MapStr{ + "msg": `{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumberAsString": "1475026826760", + "someNumber": 1475026826760, + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": "{\"a\":\"b\"}" + }`, + } + + testConfig, _ = common.NewConfigFrom(map[string]interface{}{ + "fields": fields, + "max_depth": 1, + }) + + actual := getActualValue(t, testConfig, input) + + expected := common.MapStr{ + "msg": common.MapStr{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumber": 1475026826760, + "someNumberAsString": "1475026826760", + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": "{\"a\":\"b\"}", + }, + } + + assert.Equal(t, expected.String(), actual.String()) +} + +func TestNotJsonObjectOrArrayDepthTen(t *testing.T) { + input := common.MapStr{ + "msg": `{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumberAsString": "1475026826760", + "someNumber": 1475026826760, + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": "{\"a\":\"b\"}" + }`, + } + + testConfig, _ = common.NewConfigFrom(map[string]interface{}{ + "fields": fields, + "max_depth": 10, + }) + + actual := getActualValue(t, testConfig, input) + + expected := common.MapStr{ + "msg": common.MapStr{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumber": 1475026826760, + "someNumberAsString": "1475026826760", + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": common.MapStr{"a": "b"}, + }, + } + + assert.Equal(t, expected.String(), actual.String()) +} + func getActualValue(t *testing.T, config *common.Config, input common.MapStr) common.MapStr { logp.TestingSetup() From cf94c09c6ac0030820167c8859d8aa61d4e63da5 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 19 Mar 2019 16:35:16 +0100 Subject: [PATCH 2/6] changelog --- CHANGELOG.next.asciidoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index edd32bcca81..ac4c1a2e693 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -34,6 +34,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - On Google Cloud Engine (GCE) the add_cloud_metadata will now trim the project info from the cloud.machine.type and cloud.availability_zone. {issue}10968[10968] - Rename `migration.enabled` config to `migration.6_to_7.enabled`. {pull}11284[11284] +- decode_json_field: process objects and arrays only {pull}11312[11312] *Auditbeat* @@ -170,7 +171,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix a bug when converting NetFlow fields to snake_case. {pull}10950[10950] - Add on_failure handler for Zeek ingest pipelines. Fix one field name error for notice and add an additional test case. {issue}11004[11004] {pull}11105[11105] - Fix issue preventing docker container events to be stored if the container has a network interface without ip address. {issue}11225[11225] {pull}11247[11247] -- Add on_failure handler for Zeek ingest pipelines. Fix one field name error for notice and add an additional test +- Add on_failure handler for Zeek ingest pipelines. Fix one field name error for notice and add an additional test case. {issue}11004[11004] {pull}11105[11105] - Change URLPATH grok pattern to support brackets. {issue}11135[11135] {pull}11252[11252] - Add support for iis log with different address format. {issue}11255[11255] {pull}11256[11256] From acc9bbc1e8ca7e15a2d155b0d358dcdecf6975c8 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 21 Mar 2019 08:42:34 +0100 Subject: [PATCH 3/6] table tests for depth --- .../actions/decode_json_fields_test.go | 113 ++++++++---------- 1 file changed, 53 insertions(+), 60 deletions(-) diff --git a/libbeat/processors/actions/decode_json_fields_test.go b/libbeat/processors/actions/decode_json_fields_test.go index 7a5c7a2ac78..b467b31c60e 100644 --- a/libbeat/processors/actions/decode_json_fields_test.go +++ b/libbeat/processors/actions/decode_json_fields_test.go @@ -18,6 +18,7 @@ package actions import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -199,70 +200,62 @@ func TestTargetRootOption(t *testing.T) { assert.Equal(t, expected.String(), actual.String()) } -func TestNotJsonObjectOrArrayDepthOne(t *testing.T) { - input := common.MapStr{ - "msg": `{ - "someDate": "2016-09-28T01:40:26.760+0000", - "someNumberAsString": "1475026826760", - "someNumber": 1475026826760, - "someString": "foobar", - "someString2": "2017 is awesome", - "someMap": "{\"a\":\"b\"}" - }`, - } - - testConfig, _ = common.NewConfigFrom(map[string]interface{}{ - "fields": fields, - "max_depth": 1, - }) - - actual := getActualValue(t, testConfig, input) - - expected := common.MapStr{ - "msg": common.MapStr{ - "someDate": "2016-09-28T01:40:26.760+0000", - "someNumber": 1475026826760, - "someNumberAsString": "1475026826760", - "someString": "foobar", - "someString2": "2017 is awesome", - "someMap": "{\"a\":\"b\"}", +func TestNotJsonObjectOrArray(t *testing.T) { + + var cases = []struct { + MaxDepth int + Expected common.MapStr + }{ + { + MaxDepth: 1, + Expected: common.MapStr{ + "msg": common.MapStr{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumber": 1475026826760, + "someNumberAsString": "1475026826760", + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": "{\"a\":\"b\"}", + }, + }, }, - } - - assert.Equal(t, expected.String(), actual.String()) -} - -func TestNotJsonObjectOrArrayDepthTen(t *testing.T) { - input := common.MapStr{ - "msg": `{ - "someDate": "2016-09-28T01:40:26.760+0000", - "someNumberAsString": "1475026826760", - "someNumber": 1475026826760, - "someString": "foobar", - "someString2": "2017 is awesome", - "someMap": "{\"a\":\"b\"}" - }`, - } - - testConfig, _ = common.NewConfigFrom(map[string]interface{}{ - "fields": fields, - "max_depth": 10, - }) - - actual := getActualValue(t, testConfig, input) - - expected := common.MapStr{ - "msg": common.MapStr{ - "someDate": "2016-09-28T01:40:26.760+0000", - "someNumber": 1475026826760, - "someNumberAsString": "1475026826760", - "someString": "foobar", - "someString2": "2017 is awesome", - "someMap": common.MapStr{"a": "b"}, + { + MaxDepth: 10, + Expected: common.MapStr{ + "msg": common.MapStr{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumber": 1475026826760, + "someNumberAsString": "1475026826760", + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": common.MapStr{"a": "b"}, + }, + }, }, } - assert.Equal(t, expected.String(), actual.String()) + for _, testCase := range cases { + t.Run(fmt.Sprintf("TestNotJsonObjectOrArrayDepth-%v", testCase.MaxDepth), func(t *testing.T) { + input := common.MapStr{ + "msg": `{ + "someDate": "2016-09-28T01:40:26.760+0000", + "someNumberAsString": "1475026826760", + "someNumber": 1475026826760, + "someString": "foobar", + "someString2": "2017 is awesome", + "someMap": "{\"a\":\"b\"}" + }`, + } + + testConfig, _ = common.NewConfigFrom(map[string]interface{}{ + "fields": fields, + "max_depth": testCase.MaxDepth, + }) + + actual := getActualValue(t, testConfig, input) + assert.Equal(t, testCase.Expected.String(), actual.String()) + }) + } } func getActualValue(t *testing.T, config *common.Config, input common.MapStr) common.MapStr { From 58cf87371e31863fbc828e17f6cdef1e072d62b5 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Thu, 21 Mar 2019 08:52:38 +0100 Subject: [PATCH 4/6] added array into testcase --- libbeat/processors/actions/decode_json_fields_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libbeat/processors/actions/decode_json_fields_test.go b/libbeat/processors/actions/decode_json_fields_test.go index b467b31c60e..574c9c032de 100644 --- a/libbeat/processors/actions/decode_json_fields_test.go +++ b/libbeat/processors/actions/decode_json_fields_test.go @@ -216,6 +216,7 @@ func TestNotJsonObjectOrArray(t *testing.T) { "someString": "foobar", "someString2": "2017 is awesome", "someMap": "{\"a\":\"b\"}", + "someArray": "[1,2,3]", }, }, }, @@ -229,6 +230,7 @@ func TestNotJsonObjectOrArray(t *testing.T) { "someString": "foobar", "someString2": "2017 is awesome", "someMap": common.MapStr{"a": "b"}, + "someArray": []int{1, 2, 3}, }, }, }, @@ -243,13 +245,15 @@ func TestNotJsonObjectOrArray(t *testing.T) { "someNumber": 1475026826760, "someString": "foobar", "someString2": "2017 is awesome", - "someMap": "{\"a\":\"b\"}" + "someMap": "{\"a\":\"b\"}", + "someArray": "[1,2,3]" }`, } testConfig, _ = common.NewConfigFrom(map[string]interface{}{ - "fields": fields, - "max_depth": testCase.MaxDepth, + "fields": fields, + "process_array": true, + "max_depth": testCase.MaxDepth, }) actual := getActualValue(t, testConfig, input) From ca70995c551f9ab9d5ff4e628778f33eeddb4ef7 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Fri, 22 Mar 2019 08:13:52 +0100 Subject: [PATCH 5/6] moved to fixes --- CHANGELOG.next.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 6402367668d..0484ad56312 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -35,7 +35,6 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d info from the cloud.machine.type and cloud.availability_zone. {issue}10968[10968] - Empty `meta.json` file will be treated as a missing meta file. {issue}8558[8558] - Rename `migration.enabled` config to `migration.6_to_7.enabled`. {pull}11284[11284] -- decode_json_field: process objects and arrays only {pull}11312[11312] - Beats Xpack now checks for Basic license on connect. {pull}11296[11296] *Auditbeat* @@ -147,6 +146,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Include ip and boolean type when generating index pattern. {pull}10995[10995] - Cancelling enrollment of a beat will not enroll the beat. {issue}10150[10150] - Add missing fields and test cases for libbeat add_kubernetes_metadata processor. {issue}11133[11133], {pull}11134[11134] +- decode_json_field: process objects and arrays only {pull}11312[11312] *Auditbeat* From 502caba0a2eb97b3c0b613ba25872cb0acace302 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 25 Mar 2019 13:43:28 +0100 Subject: [PATCH 6/6] added structure check & decoder error check --- .../processors/actions/decode_json_fields.go | 16 +++++++++--- .../actions/decode_json_fields_test.go | 25 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/libbeat/processors/actions/decode_json_fields.go b/libbeat/processors/actions/decode_json_fields.go index b9a8e99fcd8..74d7311f1aa 100644 --- a/libbeat/processors/actions/decode_json_fields.go +++ b/libbeat/processors/actions/decode_json_fields.go @@ -20,6 +20,7 @@ package actions import ( "encoding/json" "fmt" + "io" "strings" "github.com/pkg/errors" @@ -146,9 +147,7 @@ func unmarshal(maxDepth int, text string, fields *interface{}, processArray bool str, isString := v.(string) if !isString { return v, false - } - - if !strings.HasPrefix(str, "[") && !strings.HasPrefix(str, "{") { + } else if !isStructured(str) { return str, false } @@ -197,6 +196,10 @@ func decodeJSON(text string, to *interface{}) error { return errors.New("multiple json elements found") } + if _, err := dec.Token(); err != nil && err != io.EOF { + return err + } + switch O := interface{}(*to).(type) { case map[string]interface{}: jsontransform.TransformNumbers(O) @@ -207,3 +210,10 @@ func decodeJSON(text string, to *interface{}) error { func (f decodeJSONFields) String() string { return "decode_json_fields=" + strings.Join(f.fields, ", ") } + +func isStructured(s string) bool { + s = strings.TrimSpace(s) + end := len(s) - 1 + return end > 0 && ((s[0] == '[' && s[end] == ']') || + (s[0] == '{' && s[end] == '}')) +} diff --git a/libbeat/processors/actions/decode_json_fields_test.go b/libbeat/processors/actions/decode_json_fields_test.go index 3640ea3fa91..13aa10a00c3 100644 --- a/libbeat/processors/actions/decode_json_fields_test.go +++ b/libbeat/processors/actions/decode_json_fields_test.go @@ -260,6 +260,7 @@ func TestNotJsonObjectOrArray(t *testing.T) { }) } } + func TestArrayWithArraysDisabled(t *testing.T) { input := common.MapStr{ "msg": `{ @@ -308,6 +309,30 @@ func TestArrayWithArraysEnabled(t *testing.T) { assert.Equal(t, expected.String(), actual.String()) } +func TestArrayWithInvalidArray(t *testing.T) { + input := common.MapStr{ + "msg": `{ + "arrayOfMap": "[]]" + }`, + } + + testConfig, _ = common.NewConfigFrom(map[string]interface{}{ + "fields": fields, + "max_depth": 10, + "process_array": true, + }) + + actual := getActualValue(t, testConfig, input) + + expected := common.MapStr{ + "msg": common.MapStr{ + "arrayOfMap": "[]]", + }, + } + + assert.Equal(t, expected.String(), actual.String()) +} + func getActualValue(t *testing.T, config *common.Config, input common.MapStr) common.MapStr { logp.TestingSetup()