diff --git a/CHANGELOG.md b/CHANGELOG.md index 99b8a28907d..1718e8dfbbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - `SpanFromContext` and `SpanContextFromContext` in `go.opentelemetry.io/otel/trace` no longer make a heap allocation when the passed context has no span. (#5049) +- `NewMember` and `NewKeyValueProperty` in `go.opentelemetry.io/otel/baggage` allow percent-encoded UTF-8 string in key. (#5132) +- `NewMemberRaw`, `NewKeyProperty` and `NewKeyValuePropertyRaw` in `go.opentelemetry.io/otel/baggage` allow UTF-8 string in key. (#5132) +- `Parse` in `go.opentelemetry.io/otel/baggage` allow percent-encoded UTF-8 string in key. (#5132) - `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` now create a gRPC client in idle mode and with "dns" as the default resolver using [`grpc.NewClient`](https://pkg.go.dev/google.golang.org/grpc#NewClient). (#5151) Because of that `WithDialOption` ignores [`grpc.WithBlock`](https://pkg.go.dev/google.golang.org/grpc#WithBlock), [`grpc.WithTimeout`](https://pkg.go.dev/google.golang.org/grpc#WithTimeout), and [`grpc.WithReturnConnectionError`](https://pkg.go.dev/google.golang.org/grpc#WithReturnConnectionError). Notice that [`grpc.DialContext`](https://pkg.go.dev/google.golang.org/grpc#DialContext) which was used before is now deprecated. diff --git a/baggage/baggage.go b/baggage/baggage.go index 75773bc1ce9..4da2e889928 100644 --- a/baggage/baggage.go +++ b/baggage/baggage.go @@ -46,7 +46,7 @@ type Property struct { // // If key is invalid, an error will be returned. func NewKeyProperty(key string) (Property, error) { - if !validateKey(key) { + if !validateBaggageName(key) { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) } @@ -56,12 +56,20 @@ func NewKeyProperty(key string) (Property, error) { // NewKeyValueProperty returns a new Property for key with value. // -// The passed key must be compliant with W3C Baggage specification. +// The passed key must be percent-encoded. // The passed value must be percent-encoded as defined in W3C Baggage specification. // // Notice: Consider using [NewKeyValuePropertyRaw] instead -// that does not require percent-encoding of the value. +// that does not require percent-encoding of the key and the value. func NewKeyValueProperty(key, value string) (Property, error) { + if !validateKey(key) { + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) + } + decodedKey, err := url.PathUnescape(key) + if err != nil { + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) + } + if !validateValue(value) { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value) } @@ -69,16 +77,24 @@ func NewKeyValueProperty(key, value string) (Property, error) { if err != nil { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value) } - return NewKeyValuePropertyRaw(key, decodedValue) + return NewKeyValuePropertyRaw(decodedKey, decodedValue) } // NewKeyValuePropertyRaw returns a new Property for key with value. // -// The passed key must be compliant with W3C Baggage specification. +// The passed key and value must be valid UTF-8 string. +// However, the specific Propagators that are used to transmit baggage entries across +// component boundaries may impose their own restrictions on Property key. +// For example, the W3C Baggage specification restricts the Property keys to strings that +// satisfy the token definition from RFC7230, Section 3.2.6. +// For maximum compatibility, alpha-numeric value are strongly recommended to be used as Property key. func NewKeyValuePropertyRaw(key, value string) (Property, error) { - if !validateKey(key) { + if !validateBaggageName(key) { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) } + if !validateBaggageValue(value) { + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value) + } p := Property{ key: key, @@ -115,12 +131,15 @@ func (p Property) validate() error { return fmt.Errorf("invalid property: %w", err) } - if !validateKey(p.key) { + if !validateBaggageName(p.key) { return errFunc(fmt.Errorf("%w: %q", errInvalidKey, p.key)) } if !p.hasValue && p.value != "" { return errFunc(errors.New("inconsistent value")) } + if p.hasValue && !validateBaggageValue(p.value) { + return errFunc(fmt.Errorf("%w: %q", errInvalidValue, p.value)) + } return nil } @@ -136,13 +155,12 @@ func (p Property) Value() (string, bool) { return p.value, p.hasValue } -// String encodes Property into a header string compliant with the W3C Baggage -// specification. +// String encodes Property into a header string. func (p Property) String() string { if p.hasValue { - return fmt.Sprintf("%s%s%v", p.key, keyValueDelimiter, valueEscape(p.value)) + return fmt.Sprintf("%s%s%v", valueEscape(p.key), keyValueDelimiter, valueEscape(p.value)) } - return p.key + return valueEscape(p.key) } type properties []Property @@ -224,12 +242,20 @@ type Member struct { // NewMember returns a new Member from the passed arguments. // -// The passed key must be compliant with W3C Baggage specification. +// The passed key must be percent-encoded. // The passed value must be percent-encoded as defined in W3C Baggage specification. // // Notice: Consider using [NewMemberRaw] instead // that does not require percent-encoding of the value. func NewMember(key, value string, props ...Property) (Member, error) { + if !validateKey(key) { + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key) + } + decodedKey, err := url.PathUnescape(key) + if err != nil { + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key) + } + if !validateValue(value) { return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value) } @@ -237,12 +263,17 @@ func NewMember(key, value string, props ...Property) (Member, error) { if err != nil { return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value) } - return NewMemberRaw(key, decodedValue, props...) + return NewMemberRaw(decodedKey, decodedValue, props...) } // NewMemberRaw returns a new Member from the passed arguments. // -// The passed key must be compliant with W3C Baggage specification. +// The passed key and value must be valid UTF-8 string. +// However, the specific Propagators that are used to transmit baggage entries across +// component boundaries may impose their own restrictions on baggage key. +// For example, the W3C Baggage specification restricts the baggage keys to strings that +// satisfy the token definition from RFC7230, Section 3.2.6. +// For maximum compatibility, alpha-numeric value are strongly recommended to be used as baggage key. func NewMemberRaw(key, value string, props ...Property) (Member, error) { m := Member{ key: key, @@ -299,12 +330,18 @@ func parseMember(member string) (Member, error) { return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, v) } + // Decode a percent-encoded key. + unescapedKey, err := url.PathUnescape(key) + if err != nil { + return newInvalidMember(), fmt.Errorf("%w: %v", errInvalidKey, err) + } + // Decode a percent-encoded value. - value, err := url.PathUnescape(val) + unescapedValue, err := url.PathUnescape(val) if err != nil { return newInvalidMember(), fmt.Errorf("%w: %v", errInvalidValue, err) } - return Member{key: key, value: value, properties: props, hasData: true}, nil + return Member{key: unescapedKey, value: unescapedValue, properties: props, hasData: true}, nil } // validate ensures m conforms to the W3C Baggage specification. @@ -314,9 +351,12 @@ func (m Member) validate() error { return fmt.Errorf("%w: %q", errInvalidMember, m) } - if !validateKey(m.key) { + if !validateBaggageName(m.key) { return fmt.Errorf("%w: %q", errInvalidKey, m.key) } + if !validateBaggageValue(m.value) { + return fmt.Errorf("%w: %q", errInvalidValue, m.value) + } return m.properties.validate() } @@ -332,10 +372,8 @@ func (m Member) Properties() []Property { return m.properties.Copy() } // String encodes Member into a header string compliant with the W3C Baggage // specification. func (m Member) String() string { - // A key is just an ASCII string. A value is restricted to be - // US-ASCII characters excluding CTLs, whitespace, - // DQUOTE, comma, semicolon, and backslash. - s := fmt.Sprintf("%s%s%s", m.key, keyValueDelimiter, valueEscape(m.value)) + // A key is can be a valid UTF-8 string. + s := fmt.Sprintf("%s%s%s", valueEscape(m.key), keyValueDelimiter, valueEscape(m.value)) if len(m.properties) > 0 { s = fmt.Sprintf("%s%s%s", s, propertyDelimiter, m.properties.String()) } @@ -384,8 +422,10 @@ func New(members ...Member) (Baggage, error) { } // Parse attempts to decode a baggage-string from the passed string. It -// returns an error if the input is invalid according to the W3C Baggage +// returns an error if the input is invalid according to the OpenTelemetry Baggage // specification. +// The OpenTelemetry Baggage specification has less strict requirements than the +// W3C Baggage specification, as it allows UTF-8 characters in keys. // // If there are duplicate list-members contained in baggage, the last one // defined (reading left-to-right) will be the only one kept. This diverges @@ -606,6 +646,12 @@ func parsePropertyInternal(s string) (p Property, ok bool) { return } + // Decode a percent-encoded key. + key, err := url.PathUnescape(s[keyStart:keyEnd]) + if err != nil { + return + } + // Decode a percent-encoded value. value, err := url.PathUnescape(s[valueStart:valueEnd]) if err != nil { @@ -613,7 +659,7 @@ func parsePropertyInternal(s string) (p Property, ok bool) { } ok = true - p.key = s[keyStart:keyEnd] + p.key = key p.hasValue = true p.value = value @@ -720,6 +766,26 @@ var safeKeyCharset = [utf8.RuneSelf]bool{ '~': true, } +// validateBaggageName checks if the string is a valid OpenTelemetry Baggage name. +// Baggage name is a valid UTF-8 string. +func validateBaggageName(s string) bool { + if len(s) == 0 { + return false + } + + return utf8.ValidString(s) +} + +// validateBaggageValue checks if the string is a valid OpenTelemetry Baggage value. +// Baggage value is a valid UTF-8 strings. +func validateBaggageValue(s string) bool { + if len(s) == 0 { + return false + } + + return utf8.ValidString(s) +} + func validateKey(s string) bool { if len(s) == 0 { return false @@ -738,6 +804,7 @@ func validateKeyChar(c int32) bool { return c >= 0 && c <= int32(utf8.RuneSelf) && safeKeyCharset[c] } +// validateValue checks if the string is a valid W3C Baggage value. func validateValue(s string) bool { for _, c := range s { if !validateValueChar(c) { diff --git a/baggage/baggage_test.go b/baggage/baggage_test.go index e8f67761f0b..80d8429f00b 100644 --- a/baggage/baggage_test.go +++ b/baggage/baggage_test.go @@ -123,12 +123,22 @@ func TestParsePropertyError(t *testing.T) { func TestNewKeyProperty(t *testing.T) { p, err := NewKeyProperty(" ") - assert.ErrorIs(t, err, errInvalidKey) - assert.Equal(t, Property{}, p) + assert.NoError(t, err) + assert.Equal(t, Property{key: " "}, p) p, err = NewKeyProperty("key") assert.NoError(t, err) assert.Equal(t, Property{key: "key"}, p) + + // UTF-8 key + p, err = NewKeyProperty("B% 💼") + assert.NoError(t, err) + assert.Equal(t, Property{key: "B% 💼"}, p) + + // Invalid UTF-8 key + p, err = NewKeyProperty(string([]byte{255})) + assert.ErrorIs(t, err, errInvalidKey) + assert.Equal(t, Property{}, p) } func TestNewKeyValueProperty(t *testing.T) { @@ -140,19 +150,45 @@ func TestNewKeyValueProperty(t *testing.T) { assert.ErrorIs(t, err, errInvalidValue) assert.Equal(t, Property{}, p) + // wrong key with wrong decoding + p, err = NewKeyValueProperty("%zzzzz", "value") + assert.ErrorIs(t, err, errInvalidKey) + assert.Equal(t, Property{}, p) + + // wrong value with wrong decoding + p, err = NewKeyValueProperty("key", "%zzzzz") + assert.ErrorIs(t, err, errInvalidValue) + assert.Equal(t, Property{}, p) + p, err = NewKeyValueProperty("key", "value") assert.NoError(t, err) assert.Equal(t, Property{key: "key", value: "value", hasValue: true}, p) + + // Percent-encoded key and value + p, err = NewKeyValueProperty("%C4%85%C5%9B%C4%87", "%C4%85%C5%9B%C4%87") + assert.NoError(t, err) + assert.Equal(t, Property{key: "ąść", value: "ąść", hasValue: true}, p) } func TestNewKeyValuePropertyRaw(t *testing.T) { - p, err := NewKeyValuePropertyRaw(" ", "") + // Empty key + p, err := NewKeyValuePropertyRaw("", " ") assert.ErrorIs(t, err, errInvalidKey) assert.Equal(t, Property{}, p) - p, err = NewKeyValuePropertyRaw("key", "Witaj Świecie!") + // Empty value + p, err = NewKeyValuePropertyRaw(" ", "") + assert.ErrorIs(t, err, errInvalidValue) + assert.Equal(t, Property{}, p) + + // Space value + p, err = NewKeyValuePropertyRaw(" ", " ") + assert.NoError(t, err) + assert.Equal(t, Property{key: " ", value: " ", hasValue: true}, p) + + p, err = NewKeyValuePropertyRaw("B% 💼", "Witaj Świecie!") assert.NoError(t, err) - assert.Equal(t, Property{key: "key", value: "Witaj Świecie!", hasValue: true}, p) + assert.Equal(t, Property{key: "B% 💼", value: "Witaj Świecie!", hasValue: true}, p) } func TestPropertyValidate(t *testing.T) { @@ -167,6 +203,10 @@ func TestPropertyValidate(t *testing.T) { p.hasValue = true assert.NoError(t, p.validate()) + + // Invalid value + p.value = string([]byte{255}) + assert.ErrorIs(t, p.validate(), errInvalidValue) } func TestNewEmptyBaggage(t *testing.T) { @@ -409,6 +449,22 @@ func TestBaggageParse(t *testing.T) { "foo": {Value: "ąść"}, }, }, + { + name: "encoded UTF-8 string in key", + in: "a=b,%C4%85%C5%9B%C4%87=%C4%85%C5%9B%C4%87", + want: baggage.List{ + "a": {Value: "b"}, + "ąść": {Value: "ąść"}, + }, + }, + { + name: "encoded UTF-8 string in property", + in: "a=b,%C4%85%C5%9B%C4%87=%C4%85%C5%9B%C4%87;%C4%85%C5%9B%C4%87=%C4%85%C5%9B%C4%87", + want: baggage.List{ + "a": {Value: "b"}, + "ąść": {Value: "ąść", Properties: []baggage.Property{{Key: "ąść", HasValue: true, Value: "ąść"}}}, + }, + }, { name: "invalid member: empty", in: "foo=,,bar=", @@ -434,6 +490,11 @@ func TestBaggageParse(t *testing.T) { in: "foo=\\", err: errInvalidValue, }, + { + name: "invalid member: improper url encoded key", + in: "key%=val", + err: errInvalidKey, + }, { name: "invalid member: improper url encoded value", in: "key1=val%", @@ -449,11 +510,21 @@ func TestBaggageParse(t *testing.T) { in: "foo=1;key\\=v", err: errInvalidProperty, }, + { + name: "invalid property: improper url encoded key", + in: "foo=1;key%=v", + err: errInvalidProperty, + }, { name: "invalid property: invalid value", in: "foo=1;key=\\", err: errInvalidProperty, }, + { + name: "invalid property: improper url encoded value", + in: "foo=1;key=val%", + err: errInvalidProperty, + }, { name: "invalid baggage string: too large", in: tooLarge, @@ -604,6 +675,23 @@ func TestBaggageString(t *testing.T) { }, }, }, + { + name: "utf-8 key and value", + out: "%C4%85%C5%9B%C4%872=B%25%20%F0%9F%92%BC-2;yellow,%C4%85%C5%9B%C4%87=B%25%20%F0%9F%92%BC;%C4%85%C5%9B%C4%87-1=B%25%20%F0%9F%92%BC-1;%C4%85%C5%9B%C4%87-2", + baggage: baggage.List{ + "ąść": { + Value: "B% 💼", + Properties: []baggage.Property{ + {Key: "ąść-1", Value: "B% 💼-1", HasValue: true}, + {Key: "ąść-2"}, + }, + }, + "ąść2": { + Value: "B% 💼-2", + Properties: []baggage.Property{{Key: "yellow"}}, + }, + }, + }, } orderer := func(s string) string { @@ -861,6 +949,10 @@ func TestMemberValidation(t *testing.T) { m.hasData = true assert.ErrorIs(t, m.validate(), errInvalidKey) + // Invalid UTF-8 in value + m.key, m.value = "k", string([]byte{255}) + assert.ErrorIs(t, m.validate(), errInvalidValue) + m.key, m.value = "k", "\\" assert.NoError(t, m.validate()) } @@ -882,6 +974,17 @@ func TestNewMember(t *testing.T) { } assert.Equal(t, expected, m) + // wong key with wrong decoding + key = "%zzzzz" + _, err = NewMember(key, val, p) + assert.ErrorIs(t, err, errInvalidKey) + + // wrong value with invalid token + key = "k" + val = ";" + _, err = NewMember(key, val, p) + assert.ErrorIs(t, err, errInvalidValue) + // wrong value with wrong decoding val = "%zzzzz" _, err = NewMember(key, val, p) @@ -926,6 +1029,31 @@ func TestNewMemberRaw(t *testing.T) { assert.Equal(t, expected, m) } +func TestBaggageUTF8(t *testing.T) { + testCases := map[string]string{ + "ąść": "B% 💼", + + // Case sensitive + "a": "a", + "A": "A", + } + + var members []Member + for k, v := range testCases { + m, err := NewMemberRaw(k, v) + require.NoError(t, err) + + members = append(members, m) + } + + b, err := New(members...) + require.NoError(t, err) + + for k, v := range testCases { + assert.Equal(t, v, b.Member(k).Value()) + } +} + func TestPropertiesValidate(t *testing.T) { p := properties{{}} assert.ErrorIs(t, p.validate(), errInvalidKey) @@ -993,7 +1121,7 @@ func BenchmarkString(b *testing.B) { addMember("key1", "val1") addMember("key2", " ;,%") - addMember("key3", "Witaj świecie!") + addMember("B% 💼", "Witaj świecie!") addMember("key4", strings.Repeat("Hello world!", 10)) bg, err := New(members...)