From bfb60fa095cb69f84012a2ec7794b4dd46a08ef4 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Fri, 5 Jan 2024 15:40:57 -0800 Subject: [PATCH] wavefrontreceiver: Return error if partially quoted (#30315) Signed-off-by: Bogdan Drutu --- .chloggen/fixpartialquote.yaml | 22 ++++++ .../wavefrontreceiver/wavefront_parser.go | 75 ++++++++++--------- .../wavefront_parser_test.go | 25 ++++++- 3 files changed, 84 insertions(+), 38 deletions(-) create mode 100755 .chloggen/fixpartialquote.yaml diff --git a/.chloggen/fixpartialquote.yaml b/.chloggen/fixpartialquote.yaml new file mode 100755 index 000000000000..6aa1537b37a1 --- /dev/null +++ b/.chloggen/fixpartialquote.yaml @@ -0,0 +1,22 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'bug_fix' + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: wavefrontreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Return error if partially quoted" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [30315] + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/receiver/wavefrontreceiver/wavefront_parser.go b/receiver/wavefrontreceiver/wavefront_parser.go index 7f4432d11f3c..7cc76eb8e8a8 100644 --- a/receiver/wavefrontreceiver/wavefront_parser.go +++ b/receiver/wavefrontreceiver/wavefront_parser.go @@ -4,6 +4,7 @@ package wavefrontreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/wavefrontreceiver" import ( + "errors" "fmt" "strconv" "strings" @@ -23,8 +24,8 @@ type WavefrontParser struct { ExtractCollectdTags bool `mapstructure:"extract_collectd_tags"` } -var _ (protocol.Parser) = (*WavefrontParser)(nil) -var _ (protocol.ParserConfig) = (*WavefrontParser)(nil) +var _ protocol.Parser = (*WavefrontParser)(nil) +var _ protocol.ParserConfig = (*WavefrontParser)(nil) // Only two chars can be espcaped per Wavafront SDK, see // https://github.com/wavefrontHQ/wavefront-sdk-go/blob/2c5891318fcd83c35c93bba2b411640495473333/senders/formatter.go#L20 @@ -82,9 +83,8 @@ func (wp *WavefrontParser) Parse(line string) (pmetric.Metric, error) { attributes := pcommon.NewMap() if tags != "" { - // to need for special treatment for source, treat it as a normal tag since + // no need for special treatment for source, treat it as a normal tag since // tags are separated by space and are optionally double-quoted. - if err := buildLabels(attributes, tags); err != nil { return pmetric.Metric{}, fmt.Errorf("invalid wavefront metric [%s]: %w", line, err) } @@ -136,61 +136,64 @@ func (wp *WavefrontParser) injectCollectDLabels( return metricName } -func buildLabels(attributes pcommon.Map, tags string) (err error) { - if tags == "" { - return - } +func buildLabels(attributes pcommon.Map, tags string) error { for { - parts := strings.SplitN(tags, "=", 2) - if len(parts) != 2 { + tags = strings.TrimLeft(tags, " ") + if len(tags) == 0 { + return nil + } + + // First we need to find the key, find first '=' + keyEnd := strings.IndexByte(tags, '=') + if keyEnd == -1 { return fmt.Errorf("failed to break key for [%s]", tags) } + key := tags[:keyEnd] - key := parts[0] - rest := parts[1] - tagLen := len(key) + 1 // Length of key plus separator and yet to be determined length of the value. - var value string - if len(rest) > 1 && rest[0] == '"' { - // Skip until non-escaped double quote. + tags = tags[keyEnd+1:] + if len(tags) > 1 && tags[0] == '"' { + // Quoted value, skip until non-escaped double quote. + foundEnd := false foundEscape := false - i := 1 - for ; i < len(rest); i++ { - if rest[i] != '"' && rest[i] != 'n' { + valueEnd := 1 + for ; valueEnd < len(tags); valueEnd++ { + if tags[valueEnd] != '"' && tags[valueEnd] != 'n' { continue } - isPrevCharEscape := rest[i-1] == '\\' - if rest[i] == '"' && !isPrevCharEscape { + isPrevCharEscape := tags[valueEnd-1] == '\\' + if tags[valueEnd] == '"' && !isPrevCharEscape { // Non-escaped double-quote, it is the end of the value. + foundEnd = true break } foundEscape = foundEscape || isPrevCharEscape } - value = rest[1:i] - tagLen += len(value) + 2 // plus 2 to account for the double-quotes. + // If we didn't find non-escaped double-quote then this is an error. + if !foundEnd { + return errors.New("partially quoted tag value") + } + + value := tags[1:valueEnd] + tags = tags[valueEnd+1:] if foundEscape { // Per implementation of Wavefront SDK only double-quotes and // newline characters are escaped. See the link below: // https://github.com/wavefrontHQ/wavefront-sdk-go/blob/2c5891318fcd83c35c93bba2b411640495473333/senders/formatter.go#L20 value = escapedCharReplacer.Replace(value) } + attributes.PutStr(key, value) } else { - // Skip until space. - i := 0 - for ; i < len(rest) && rest[i] != ' '; i++ { // nolint + valueEnd := strings.IndexByte(tags, ' ') + if valueEnd == -1 { + // The value is up to the end. + attributes.PutStr(key, tags) + return nil } - value = rest[:i] - tagLen += i - } - attributes.PutStr(key, value) - - tags = strings.TrimLeft(tags[tagLen:], " ") - if tags == "" { - break + attributes.PutStr(key, tags[:valueEnd]) + tags = tags[valueEnd+1:] } } - - return } func unDoubleQuote(s string) string { diff --git a/receiver/wavefrontreceiver/wavefront_parser_test.go b/receiver/wavefrontreceiver/wavefront_parser_test.go index 20aca6cc4b1a..22351e98c6ac 100644 --- a/receiver/wavefrontreceiver/wavefront_parser_test.go +++ b/receiver/wavefrontreceiver/wavefront_parser_test.go @@ -46,10 +46,10 @@ func Test_buildLabels(t *testing.T) { }, { name: "end_with_quotes", - tags: "source=\"tst escape\\\" tst\" x=\"tst spc\"", + tags: "source=\"test escaped\\\" text\" x=\"tst spc\"", wantAttributes: func() pcommon.Map { m := pcommon.NewMap() - m.PutStr("source", "tst escape\" tst") + m.PutStr("source", "test escaped\" text") m.PutStr("x", "tst spc") return m }(), @@ -79,6 +79,17 @@ func Test_buildLabels(t *testing.T) { }, { name: "empty_tagValue", + tags: "k0=0 k1=" + + "", + wantAttributes: func() pcommon.Map { + m := pcommon.NewMap() + m.PutStr("k0", "0") + m.PutStr("k1", "") + return m + }(), + }, + { + name: "empty_quoted_tagValue", tags: "k0=0 k1=\"\"", wantAttributes: func() pcommon.Map { m := pcommon.NewMap() @@ -92,6 +103,16 @@ func Test_buildLabels(t *testing.T) { tags: "k0=0 k1", wantErr: true, }, + { + name: "partially_quoted", + tags: "k0=0 k1=\"test", + wantErr: true, + }, + { + name: "end_with_escaped_", + tags: "k0=0 k1=\"test\\\"", + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {