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 5346ed8e976..b8ebe8388c2 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 fbc6b5b3014..afaa0ac59d7 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 2469fe1848c..fa82a3de132 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 b8ebe8388c2..3a5be54b8c7 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 fa82a3de132..141fafc0f8f 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 afaa0ac59d7..90204794211 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 90204794211..887fbdd02a9 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)