Stop percent-encoding the header environment variables in otlplog exporters (#6392)

Bugfixes for #5623 Based on the conversation https://github.com/open-telemetry/opentelemetry-go/issues/5623#issuecomment-2331089315, only OTLP log exporters need bugfixes. --------- Co-authored-by: Damien Mathieu <42@dmathieu.com> Co-authored-by: Robert Pająk <pellared@hotmail.com>
2025-08-10 22:31:50 +02:00 · 2025-03-12 01:32:18 -04:00
parent fb89a382e6
commit efe325aa95
5 changed files with 312 additions and 16 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

 - Drop support for [Go 1.22]. (#6381, #6418)

+### Fixes
+
+- Stop percent encoding header environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6392)
+
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->

--- 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"
@@ -442,13 +443,15 @@ func convHeaders(s string) (map[string]string, error) {
 			continue
 		}

-		escKey, e := url.PathUnescape(rawKey)
-		if e != nil {
+		key := strings.TrimSpace(rawKey)
+
+		// Validate the key.
+		if !isValidHeaderKey(key) {
 			err = errors.Join(err, fmt.Errorf("invalid header key: %s", rawKey))
 			continue
 		}
-		key := strings.TrimSpace(escKey)

+		// Only decode the value.
 		escVal, e := url.PathUnescape(rawVal)
 		if e != nil {
 			err = errors.Join(err, fmt.Errorf("invalid header value: %s", rawVal))
@@ -651,3 +654,22 @@ func fallback[T any](val T) resolver[T] {
 		return s
 	}
 }
+
+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 == '~')
+}
--- a/exporters/otlp/otlplog/otlploggrpc/config_test.go
+++ b/exporters/otlp/otlplog/otlploggrpc/config_test.go
@@ -338,7 +338,7 @@ func TestNewConfig(t *testing.T) {
 			name: "InvalidEnvironmentVariables",
 			envars: map[string]string{
 				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT":           "%invalid",
-				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "a,%ZZ=valid,key=%ZZ",
+				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "invalid key=value",
 				"OTEL_EXPORTER_OTLP_LOGS_COMPRESSION":        "xz",
 				"OTEL_EXPORTER_OTLP_LOGS_TIMEOUT":            "100 seconds",
 				"OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE":        "invalid_cert",
@@ -355,10 +355,7 @@ func TestNewConfig(t *testing.T) {
 				`failed to load TLS:`,
 				`certificate not added`,
 				`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_HEADERS value invalid key=value: invalid header key: invalid key`,
 				`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`,
 			},
@@ -440,6 +437,47 @@ func TestNewConfig(t *testing.T) {
 				timeout:  newSetting(defaultTimeout),
 			},
 		},
+		{
+			name: "with percent-encoded headers",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT":           "https://env.endpoint:8080/prefix",
+				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "user%2Did=42,user%20name=alice%20smith",
+				"OTEL_EXPORTER_OTLP_LOGS_COMPRESSION":        "gzip",
+				"OTEL_EXPORTER_OTLP_LOGS_TIMEOUT":            "15000",
+				"OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE":        "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_CERTIFICATE": "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_KEY":         "key_path",
+			},
+			want: config{
+				endpoint:    newSetting("env.endpoint:8080"),
+				insecure:    newSetting(false),
+				tlsCfg:      newSetting(tlsCfg),
+				headers:     newSetting(map[string]string{"user%2Did": "42", "user%20name": "alice smith"}),
+				compression: newSetting(GzipCompression),
+				timeout:     newSetting(15 * time.Second),
+				retryCfg:    newSetting(defaultRetryCfg),
+			},
+		},
+		{
+			name: "with invalid header key",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT":           "https://env.endpoint:8080/prefix",
+				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "valid-key=value,invalid key=value",
+				"OTEL_EXPORTER_OTLP_LOGS_COMPRESSION":        "gzip",
+				"OTEL_EXPORTER_OTLP_LOGS_TIMEOUT":            "15000",
+				"OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE":        "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_CERTIFICATE": "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_KEY":         "key_path",
+			},
+			want: config{
+				endpoint:    newSetting("env.endpoint:8080"),
+				insecure:    newSetting(false),
+				tlsCfg:      newSetting(tlsCfg),
+				compression: newSetting(GzipCompression),
+				timeout:     newSetting(15 * time.Second),
+				retryCfg:    newSetting(defaultRetryCfg),
+			},
+		},
 	}

 	for _, tc := range testcases {
@@ -494,3 +532,88 @@ func assertTLSConfig(t *testing.T, want, got setting[*tls.Config]) {
 	}
 	assert.Equal(t, want.Value.Certificates, got.Value.Certificates, "Certificates")
 }
+
+func TestConvHeaders(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   string
+		want    map[string]string
+		wantErr bool
+	}{
+		{
+			name:    "simple test",
+			value:   "userId=alice",
+			want:    map[string]string{"userId": "alice"},
+			wantErr: false,
+		},
+		{
+			name:    "simple test with spaces",
+			value:   " userId = alice  ",
+			want:    map[string]string{"userId": "alice"},
+			wantErr: false,
+		},
+		{
+			name:    "simple header conforms to RFC 3986 spec",
+			value:   " userId = alice+test ",
+			want:    map[string]string{"userId": "alice+test"},
+			wantErr: false,
+		},
+		{
+			name:  "multiple headers encoded",
+			value: "userId=alice,serverNode=DF%3A28,isProduction=false",
+			want: map[string]string{
+				"userId":       "alice",
+				"serverNode":   "DF:28",
+				"isProduction": "false",
+			},
+			wantErr: false,
+		},
+		{
+			name:  "multiple headers encoded per RFC 3986 spec",
+			value: "userId=alice+test,serverNode=DF%3A28,isProduction=false,namespace=localhost/test",
+			want: map[string]string{
+				"userId":       "alice+test",
+				"serverNode":   "DF:28",
+				"isProduction": "false",
+				"namespace":    "localhost/test",
+			},
+			wantErr: false,
+		},
+		{
+			name:    "invalid headers format",
+			value:   "userId:alice",
+			want:    map[string]string{},
+			wantErr: true,
+		},
+		{
+			name:  "invalid key",
+			value: "%XX=missing,userId=alice",
+			want: map[string]string{
+				"%XX":    "missing",
+				"userId": "alice",
+			},
+			wantErr: false,
+		},
+		{
+			name:  "invalid value",
+			value: "missing=%XX,userId=alice",
+			want: map[string]string{
+				"userId": "alice",
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			keyValues, err := convHeaders(tt.value)
+			assert.Equal(t, tt.want, keyValues)
+
+			if tt.wantErr {
+				assert.Error(t, err, "expected an error but got nil")
+			} else {
+				assert.NoError(t, err, "expected no error but got one")
+			}
+		})
+	}
+}
--- 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"
@@ -544,13 +545,15 @@ func convHeaders(s string) (map[string]string, error) {
 			continue
 		}

-		escKey, e := url.PathUnescape(rawKey)
-		if e != nil {
+		key := strings.TrimSpace(rawKey)
+
+		// Validate the key.
+		if !isValidHeaderKey(key) {
 			err = errors.Join(err, fmt.Errorf("invalid header key: %s", rawKey))
 			continue
 		}
-		key := strings.TrimSpace(escKey)

+		// Only decode the value.
 		escVal, e := url.PathUnescape(rawVal)
 		if e != nil {
 			err = errors.Join(err, fmt.Errorf("invalid header value: %s", rawVal))
@@ -600,3 +603,22 @@ func fallback[T any](val T) resolver[T] {
 		return s
 	}
 }
+
+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 == '~')
+}
--- a/exporters/otlp/otlplog/otlploghttp/config_test.go
+++ b/exporters/otlp/otlplog/otlploghttp/config_test.go
@@ -344,7 +344,7 @@ func TestNewConfig(t *testing.T) {
 			name: "InvalidEnvironmentVariables",
 			envars: map[string]string{
 				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT":           "%invalid",
-				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "a,%ZZ=valid,key=%ZZ",
+				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "invalid key=value",
 				"OTEL_EXPORTER_OTLP_LOGS_COMPRESSION":        "xz",
 				"OTEL_EXPORTER_OTLP_LOGS_TIMEOUT":            "100 seconds",
 				"OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE":        "invalid_cert",
@@ -362,14 +362,54 @@ func TestNewConfig(t *testing.T) {
 				`failed to load TLS:`,
 				`certificate not added`,
 				`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_HEADERS value invalid key=value: invalid header key: invalid key`,
 				`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: "with percent-encoded headers",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT":           "https://env.endpoint:8080/prefix",
+				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "user%2Did=42,user%20name=alice%20smith",
+				"OTEL_EXPORTER_OTLP_LOGS_COMPRESSION":        "gzip",
+				"OTEL_EXPORTER_OTLP_LOGS_TIMEOUT":            "15000",
+				"OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE":        "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_CERTIFICATE": "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_KEY":         "key_path",
+			},
+			want: config{
+				endpoint:    newSetting("env.endpoint:8080"),
+				path:        newSetting("/prefix"),
+				insecure:    newSetting(false),
+				tlsCfg:      newSetting(tlsCfg),
+				headers:     newSetting(map[string]string{"user%2Did": "42", "user%20name": "alice smith"}),
+				compression: newSetting(GzipCompression),
+				timeout:     newSetting(15 * time.Second),
+				retryCfg:    newSetting(defaultRetryCfg),
+			},
+		},
+		{
+			name: "with invalid header key",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT":           "https://env.endpoint:8080/prefix",
+				"OTEL_EXPORTER_OTLP_LOGS_HEADERS":            "valid-key=value,invalid key=value",
+				"OTEL_EXPORTER_OTLP_LOGS_COMPRESSION":        "gzip",
+				"OTEL_EXPORTER_OTLP_LOGS_TIMEOUT":            "15000",
+				"OTEL_EXPORTER_OTLP_LOGS_CERTIFICATE":        "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_CERTIFICATE": "cert_path",
+				"OTEL_EXPORTER_OTLP_LOGS_CLIENT_KEY":         "key_path",
+			},
+			want: config{
+				endpoint:    newSetting("env.endpoint:8080"),
+				path:        newSetting("/prefix"),
+				insecure:    newSetting(false),
+				tlsCfg:      newSetting(tlsCfg),
+				compression: newSetting(GzipCompression),
+				timeout:     newSetting(15 * time.Second),
+				retryCfg:    newSetting(defaultRetryCfg),
+			},
+		},
 	}

 	for _, tc := range testcases {
@@ -436,3 +476,88 @@ func TestWithProxy(t *testing.T) {
 	assert.True(t, c.proxy.Set)
 	assert.NotNil(t, c.proxy.Value)
 }
+
+func TestConvHeaders(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   string
+		want    map[string]string
+		wantErr bool
+	}{
+		{
+			name:    "simple test",
+			value:   "userId=alice",
+			want:    map[string]string{"userId": "alice"},
+			wantErr: false,
+		},
+		{
+			name:    "simple test with spaces",
+			value:   " userId = alice  ",
+			want:    map[string]string{"userId": "alice"},
+			wantErr: false,
+		},
+		{
+			name:    "simple header conforms to RFC 3986 spec",
+			value:   " userId = alice+test ",
+			want:    map[string]string{"userId": "alice+test"},
+			wantErr: false,
+		},
+		{
+			name:  "multiple headers encoded",
+			value: "userId=alice,serverNode=DF%3A28,isProduction=false",
+			want: map[string]string{
+				"userId":       "alice",
+				"serverNode":   "DF:28",
+				"isProduction": "false",
+			},
+			wantErr: false,
+		},
+		{
+			name:  "multiple headers encoded per RFC 3986 spec",
+			value: "userId=alice+test,serverNode=DF%3A28,isProduction=false,namespace=localhost/test",
+			want: map[string]string{
+				"userId":       "alice+test",
+				"serverNode":   "DF:28",
+				"isProduction": "false",
+				"namespace":    "localhost/test",
+			},
+			wantErr: false,
+		},
+		{
+			name:    "invalid headers format",
+			value:   "userId:alice",
+			want:    map[string]string{},
+			wantErr: true,
+		},
+		{
+			name:  "invalid key",
+			value: "%XX=missing,userId=alice",
+			want: map[string]string{
+				"%XX":    "missing",
+				"userId": "alice",
+			},
+			wantErr: false,
+		},
+		{
+			name:  "invalid value",
+			value: "missing=%XX,userId=alice",
+			want: map[string]string{
+				"userId": "alice",
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			keyValues, err := convHeaders(tt.value)
+			assert.Equal(t, tt.want, keyValues)
+
+			if tt.wantErr {
+				assert.Error(t, err, "expected an error but got nil")
+			} else {
+				assert.NoError(t, err, "expected no error but got one")
+			}
+		})
+	}
+}