From 4dcb2f5dbb215dc4ade8aafe0cac2e4a08321549 Mon Sep 17 00:00:00 2001 From: Zhihan Li Date: Fri, 6 Sep 2024 14:09:07 +0100 Subject: [PATCH 1/2] bugfix 5705 --- exporters/otlp/otlplog/otlploggrpc/config.go | 29 ++++++++++-- .../otlp/otlplog/otlploggrpc/config_test.go | 42 ++++++++++++++++- exporters/otlp/otlplog/otlploghttp/config.go | 29 ++++++++++-- .../otlp/otlplog/otlploghttp/config_test.go | 45 ++++++++++++++++++- 4 files changed, 135 insertions(+), 10 deletions(-) diff --git a/exporters/otlp/otlplog/otlploggrpc/config.go b/exporters/otlp/otlplog/otlploggrpc/config.go index 52e8c256c6c..fce5229c11a 100644 --- a/exporters/otlp/otlplog/otlploggrpc/config.go +++ b/exporters/otlp/otlplog/otlploggrpc/config.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" "time" + "unicode" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -418,12 +419,13 @@ func convHeaders(s string) (map[string]string, error) { continue } - escKey, e := url.PathUnescape(rawKey) - if e != nil { + trimmedKey := strings.TrimSpace(rawKey) + + // Validate the key. + if !isValidHeaderKey(trimmedKey) { err = errors.Join(err, fmt.Errorf("invalid header key: %s", rawKey)) continue } - key := strings.TrimSpace(escKey) escVal, e := url.PathUnescape(rawVal) if e != nil { @@ -432,11 +434,30 @@ func convHeaders(s string) (map[string]string, error) { } val := strings.TrimSpace(escVal) - out[key] = val + out[trimmedKey] = val } return out, err } +func isValidHeaderKey(key string) bool { + if key == "" { + return false + } + for _, c := range key { + if !isTokenChar(c) { + return false + } + } + return true +} + +func isTokenChar(c rune) bool { + return c <= unicode.MaxASCII && (unicode.IsLetter(c) || + unicode.IsDigit(c) || + c == '!' || c == '#' || c == '$' || c == '%' || c == '&' || c == '\'' || c == '*' || + c == '+' || c == '-' || c == '.' || c == '^' || c == '_' || c == '`' || c == '|' || c == '~') +} + // convDuration converts s into a duration of milliseconds. If s does not // contain an integer, 0 and an error are returned. func convDuration(s string) (time.Duration, error) { diff --git a/exporters/otlp/otlplog/otlploggrpc/config_test.go b/exporters/otlp/otlplog/otlploggrpc/config_test.go index 02817476f5c..c71482bd44c 100644 --- a/exporters/otlp/otlplog/otlploggrpc/config_test.go +++ b/exporters/otlp/otlplog/otlploggrpc/config_test.go @@ -327,12 +327,52 @@ func TestNewConfig(t *testing.T) { `tls: failed to find any PEM data in certificate input`, `invalid OTEL_EXPORTER_OTLP_LOGS_HEADERS value a,%ZZ=valid,key=%ZZ:`, `invalid header: a`, - `invalid header key: %ZZ`, `invalid header value: %ZZ`, `invalid OTEL_EXPORTER_OTLP_LOGS_COMPRESSION value xz: unknown compression: xz`, `invalid OTEL_EXPORTER_OTLP_LOGS_TIMEOUT value 100 seconds: strconv.Atoi: parsing "100 seconds": invalid syntax`, }, }, + { + name: "HeadersWithSpaces", + envars: map[string]string{ + "OTEL_EXPORTER_OTLP_HEADERS": " keyWithSpaces =value1", + }, + want: config{ + headers: newSetting(map[string]string{ + "keyWithSpaces": "value1", + }), + endpoint: newSetting(defaultEndpoint), + timeout: newSetting(defaultTimeout), + retryCfg: newSetting(defaultRetryCfg), + }, + }, + { + name: "HeadersWithPercentEncoding", + envars: map[string]string{ + "OTEL_EXPORTER_OTLP_HEADERS": "percent%20encoded=value,normal=ok", + }, + want: config{ + headers: newSetting(map[string]string{ + "percent%20encoded": "value", + "normal": "ok", + }), + endpoint: newSetting(defaultEndpoint), + timeout: newSetting(defaultTimeout), + retryCfg: newSetting(defaultRetryCfg), + }, + }, + { + name: "HeadersWithInvalidKey", + envars: map[string]string{ + "OTEL_EXPORTER_OTLP_HEADERS": "valid=ok,inva lid=not_ok,also_valid=fine", + }, + want: config{ + endpoint: newSetting(defaultEndpoint), + timeout: newSetting(defaultTimeout), + retryCfg: newSetting(defaultRetryCfg), + }, + errs: []string{`invalid header key: inva lid`}, + }, } for _, tc := range testcases { diff --git a/exporters/otlp/otlplog/otlploghttp/config.go b/exporters/otlp/otlplog/otlploghttp/config.go index 88a8d5e1f10..090c08221c6 100644 --- a/exporters/otlp/otlplog/otlploghttp/config.go +++ b/exporters/otlp/otlplog/otlploghttp/config.go @@ -14,6 +14,7 @@ import ( "strconv" "strings" "time" + "unicode" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp/internal/retry" @@ -548,12 +549,13 @@ func convHeaders(s string) (map[string]string, error) { continue } - escKey, e := url.PathUnescape(rawKey) - if e != nil { + trimmedKey := strings.TrimSpace(rawKey) + + // Validate the key. + if !isValidHeaderKey(trimmedKey) { err = errors.Join(err, fmt.Errorf("invalid header key: %s", rawKey)) continue } - key := strings.TrimSpace(escKey) escVal, e := url.PathUnescape(rawVal) if e != nil { @@ -562,11 +564,30 @@ func convHeaders(s string) (map[string]string, error) { } val := strings.TrimSpace(escVal) - out[key] = val + out[trimmedKey] = val } return out, err } +func isValidHeaderKey(key string) bool { + if key == "" { + return false + } + for _, c := range key { + if !isTokenChar(c) { + return false + } + } + return true +} + +func isTokenChar(c rune) bool { + return c <= unicode.MaxASCII && (unicode.IsLetter(c) || + unicode.IsDigit(c) || + c == '!' || c == '#' || c == '$' || c == '%' || c == '&' || c == '\'' || c == '*' || + c == '+' || c == '-' || c == '.' || c == '^' || c == '_' || c == '`' || c == '|' || c == '~') +} + // convCompression returns the parsed compression encoded in s. NoCompression // and an errors are returned if s is unknown. func convCompression(s string) (Compression, error) { diff --git a/exporters/otlp/otlplog/otlploghttp/config_test.go b/exporters/otlp/otlplog/otlploghttp/config_test.go index d3ef1cc1977..88a4ce648c4 100644 --- a/exporters/otlp/otlplog/otlploghttp/config_test.go +++ b/exporters/otlp/otlplog/otlploghttp/config_test.go @@ -332,12 +332,55 @@ func TestNewConfig(t *testing.T) { `tls: failed to find any PEM data in certificate input`, `invalid OTEL_EXPORTER_OTLP_LOGS_HEADERS value a,%ZZ=valid,key=%ZZ:`, `invalid header: a`, - `invalid header key: %ZZ`, `invalid header value: %ZZ`, `invalid OTEL_EXPORTER_OTLP_LOGS_COMPRESSION value xz: unknown compression: xz`, `invalid OTEL_EXPORTER_OTLP_LOGS_TIMEOUT value 100 seconds: strconv.Atoi: parsing "100 seconds": invalid syntax`, }, }, + { + name: "HeadersWithSpaces", + envars: map[string]string{ + "OTEL_EXPORTER_OTLP_HEADERS": " keyWithSpaces =value1", + }, + want: config{ + headers: newSetting(map[string]string{ + "keyWithSpaces": "value1", + }), + endpoint: newSetting(defaultEndpoint), + path: newSetting(defaultPath), + timeout: newSetting(defaultTimeout), + retryCfg: newSetting(defaultRetryCfg), + }, + }, + { + name: "HeadersWithPercentEncoding", + envars: map[string]string{ + "OTEL_EXPORTER_OTLP_HEADERS": "percent%20encoded=value,normal=ok", + }, + want: config{ + headers: newSetting(map[string]string{ + "percent%20encoded": "value", + "normal": "ok", + }), + endpoint: newSetting(defaultEndpoint), + path: newSetting(defaultPath), + timeout: newSetting(defaultTimeout), + retryCfg: newSetting(defaultRetryCfg), + }, + }, + { + name: "HeadersWithInvalidKey", + envars: map[string]string{ + "OTEL_EXPORTER_OTLP_HEADERS": "valid=ok,inva lid=not_ok,also_valid=fine", + }, + want: config{ + endpoint: newSetting(defaultEndpoint), + path: newSetting(defaultPath), + timeout: newSetting(defaultTimeout), + retryCfg: newSetting(defaultRetryCfg), + }, + errs: []string{`invalid header key: inva lid`}, + }, } for _, tc := range testcases { From 59041d36e71da09841a26f9ba06aa2ef00a0a93e Mon Sep 17 00:00:00 2001 From: Zhihan Li Date: Fri, 6 Sep 2024 14:20:50 +0100 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8c72034500..fe52c0c7947 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix memory leak in the global `MeterProvider` when identical instruments are repeatedly created. (#5754) - Fix panic instruments creation when setting meter provider. (#5758) +- Stop percent encoding header environment variables in `open-telemetry/opentelemetry-go/exporters/otlp/otlplog/otlploggrpc` and `open-telemetry/opentelemetry-go/exporters/otlp/otlplog/otlploghttp` (#5792) +- Remove invalid environment variable header keys in `open-telemetry/opentelemetry-go/exporters/otlp/otlplog/otlploggrpc` and `open-telemetry/opentelemetry-go/exporters/otlp/otlplog/otlploghttp` (#5792) ### Removed