From de47173b679322c42f2b7a307b741aade8027f37 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Tue, 14 Jan 2020 22:27:08 +0100 Subject: [PATCH 1/4] Fix convert processor conversion of string to integer The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes #15513 --- CHANGELOG.next.asciidoc | 1 + libbeat/processors/convert/convert.go | 14 ++++++++++++-- libbeat/processors/convert/convert_test.go | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 5346ed8e976f..b8ebe8388c28 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -151,6 +151,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix bug with potential concurrent reads and writes from event.Meta map by Kafka output. {issue}14542[14542] {pull}14568[14568] - Fix spooling to disk blocking infinitely if the lock file can not be acquired. {pull}15338[15338] - Fix `metricbeat test output` with an ipv6 ES host in the output.hosts. {pull}15368[15368] +- Fix `convert` processor conversion of string to integer with leading zeros. {issue}15513[15513] *Auditbeat* diff --git a/libbeat/processors/convert/convert.go b/libbeat/processors/convert/convert.go index fbc6b5b30146..afaa0ac59d76 100644 --- a/libbeat/processors/convert/convert.go +++ b/libbeat/processors/convert/convert.go @@ -205,7 +205,7 @@ func toString(value interface{}) (string, error) { func toLong(value interface{}) (int64, error) { switch v := value.(type) { case string: - return strconv.ParseInt(v, 0, 64) + return strToInt(v, 64) case int: return int64(v), nil case int8: @@ -238,7 +238,7 @@ func toLong(value interface{}) (int64, error) { func toInteger(value interface{}) (int32, error) { switch v := value.(type) { case string: - i, err := strconv.ParseInt(v, 0, 32) + i, err := strToInt(v, 32) return int32(i), err case int: return int32(v), nil @@ -403,3 +403,13 @@ func cloneValue(value interface{}) interface{} { return value } } + +// Helper to interpret a string as either base-10 or base-16. +func strToInt(v string, bitSize int) (int64, error) { + base := 10 + if strings.IndexAny(v, "xX") != -1 { + // strconv.ParseInt only accepts the '0x' prefix when base is 0. + base = 0 + } + return strconv.ParseInt(v, base, bitSize) +} diff --git a/libbeat/processors/convert/convert_test.go b/libbeat/processors/convert/convert_test.go index 2469fe1848c9..fa82a3de132a 100644 --- a/libbeat/processors/convert/convert_test.go +++ b/libbeat/processors/convert/convert_test.go @@ -276,8 +276,16 @@ var testCases = []testCase{ {Long, nil, nil, true}, {Long, "x", nil, true}, + {Long, "0x", nil, true}, + {Long, "0b1", nil, true}, + {Long, "1x2", nil, true}, {Long, true, nil, true}, {Long, "1", int64(1), false}, + {Long, "-1", int64(-1), false}, + {Long, "017", int64(17), false}, + {Long, "08", int64(8), false}, + {Long, "0X0A", int64(10), false}, + {Long, "-0x12", int64(-18), false}, {Long, int(1), int64(1), false}, {Long, int8(1), int64(1), false}, {Long, int16(1), int64(1), false}, @@ -294,6 +302,17 @@ var testCases = []testCase{ {Integer, nil, nil, true}, {Integer, "x", nil, true}, {Integer, true, nil, true}, + {Integer, "x", nil, true}, + {Integer, "0x", nil, true}, + {Integer, "0b1", nil, true}, + {Integer, "1x2", nil, true}, + {Integer, true, nil, true}, + {Integer, "1", int32(1), false}, + {Integer, "-1", int32(-1), false}, + {Integer, "017", int32(7), false}, + {Integer, "08", int32(8), false}, + {Integer, "0X0A", int32(10), false}, + {Integer, "-0x12", int32(-18), false}, {Integer, "1", int32(1), false}, {Integer, int(1), int32(1), false}, {Integer, int8(1), int32(1), false}, From 733b4e4a600e955a11093fed9765ecb3ec6da050 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Tue, 14 Jan 2020 23:37:15 +0100 Subject: [PATCH 2/4] Add PR number and fix test --- CHANGELOG.next.asciidoc | 2 +- libbeat/processors/convert/convert_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index b8ebe8388c28..3a5be54b8c79 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -151,7 +151,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix bug with potential concurrent reads and writes from event.Meta map by Kafka output. {issue}14542[14542] {pull}14568[14568] - Fix spooling to disk blocking infinitely if the lock file can not be acquired. {pull}15338[15338] - Fix `metricbeat test output` with an ipv6 ES host in the output.hosts. {pull}15368[15368] -- Fix `convert` processor conversion of string to integer with leading zeros. {issue}15513[15513] +- Fix `convert` processor conversion of string to integer with leading zeros. {issue}15513[15513] {pull}15557[15557] *Auditbeat* diff --git a/libbeat/processors/convert/convert_test.go b/libbeat/processors/convert/convert_test.go index fa82a3de132a..141fafc0f8fe 100644 --- a/libbeat/processors/convert/convert_test.go +++ b/libbeat/processors/convert/convert_test.go @@ -309,7 +309,7 @@ var testCases = []testCase{ {Integer, true, nil, true}, {Integer, "1", int32(1), false}, {Integer, "-1", int32(-1), false}, - {Integer, "017", int32(7), false}, + {Integer, "017", int32(17), false}, {Integer, "08", int32(8), false}, {Integer, "0X0A", int32(10), false}, {Integer, "-0x12", int32(-18), false}, From bb9436f65f160cdd52705e1be080eb18a1af805b Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Wed, 15 Jan 2020 00:09:25 +0100 Subject: [PATCH 3/4] Not so-silly hex check --- libbeat/processors/convert/convert.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/libbeat/processors/convert/convert.go b/libbeat/processors/convert/convert.go index afaa0ac59d76..902047942113 100644 --- a/libbeat/processors/convert/convert.go +++ b/libbeat/processors/convert/convert.go @@ -404,12 +404,23 @@ func cloneValue(value interface{}) interface{} { } } -// Helper to interpret a string as either base-10 or base-16. -func strToInt(v string, bitSize int) (int64, error) { +// strToInt is a helper to interpret a string as either base 10 or base 16. +func strToInt(s string, bitSize int) (int64, error) { base := 10 - if strings.IndexAny(v, "xX") != -1 { - // strconv.ParseInt only accepts the '0x' prefix when base is 0. + if hasHexPrefix(s) { + // ParseInt accepts the hex prefix when base=0, not when base=16. base = 0 } - return strconv.ParseInt(v, base, bitSize) + return strconv.ParseInt(s, base, bitSize) +} + +func hasHexPrefix(s string) bool { + if len(s) < 3 { + return false + } + a, b := s[0], s[1] + if a == '+' || a == '-' { + a, b = b, s[2] + } + return a == '0' && (b == 'x' || b == 'X') } From defd2b113b339120e7f865ca7eb39a6b71a294b5 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Wed, 15 Jan 2020 00:14:27 +0100 Subject: [PATCH 4/4] Comment --- libbeat/processors/convert/convert.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libbeat/processors/convert/convert.go b/libbeat/processors/convert/convert.go index 902047942113..887fbdd02a9f 100644 --- a/libbeat/processors/convert/convert.go +++ b/libbeat/processors/convert/convert.go @@ -408,7 +408,7 @@ func cloneValue(value interface{}) interface{} { func strToInt(s string, bitSize int) (int64, error) { base := 10 if hasHexPrefix(s) { - // ParseInt accepts the hex prefix when base=0, not when base=16. + // strconv.ParseInt will accept the '0x' or '0X` prefix only when base is 0. base = 0 } return strconv.ParseInt(s, base, bitSize)