Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

baggage: Accept non-ASCII keys #5132

Merged
merged 35 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
c81a51e
Allow UTF-8 string in baggage key
XSAM Mar 30, 2024
bc9c5b8
Merge remote-tracking branch 'upstream/main' into utf8-baggage
XSAM Apr 2, 2024
dc96def
Update CHANGELOG
XSAM Apr 2, 2024
7d9d476
Merge branch 'main' into utf8-baggage
hanyuancheung Apr 5, 2024
6501fba
Merge branch 'main' into utf8-baggage
hanyuancheung Apr 5, 2024
a037804
Merge remote-tracking branch 'upstream/main' into utf8-baggage
XSAM Apr 20, 2024
9434b2f
Allow UTF-8 in property key
XSAM Apr 20, 2024
171b0db
Fix tests
XSAM Apr 21, 2024
ce67af3
Allow UTF-8 in key to be parsed
XSAM Apr 21, 2024
e864429
Add more tests to cover 100% of lines
XSAM Apr 21, 2024
097d8a2
Update CHANGELOG
XSAM Apr 21, 2024
d716cec
Add more tests
XSAM Apr 21, 2024
c4497c1
Merge remote-tracking branch 'upstream/main' into utf8-baggage
XSAM Jun 19, 2024
a07ffb4
Allow empty string as baggage value
XSAM Jun 19, 2024
6d50ce2
Merge remote-tracking branch 'upstream/main' into utf8-baggage
XSAM Jul 3, 2024
6c289d9
Keep the behavior of W3C baggage propagator
XSAM Jul 3, 2024
6c01460
Revert the behavior of Parse to only parse W3C Baggage
XSAM Jul 3, 2024
1a88be7
Update CHANGELOG
XSAM Jul 3, 2024
7c344d6
Merge branch 'main' into utf8-baggage
XSAM Jul 16, 2024
6eed4e0
Update comments
XSAM Jul 17, 2024
ef3973e
Revert the behavior of NewMember
XSAM Jul 21, 2024
99fcf53
Revert the behavior of NewKeyValueProperty
XSAM Jul 21, 2024
8d69418
Update CHANGELOG
XSAM Jul 21, 2024
5200935
Merge remote-tracking branch 'upstream/main' into utf8-baggage
XSAM Jul 21, 2024
9a85415
Update comments
XSAM Jul 21, 2024
a544f84
Remove the key escaping in `String` method of `Member`
XSAM Jul 24, 2024
e8e07f3
Update baggage/baggage.go
pellared Jul 24, 2024
1010792
Merge branch 'main' into utf8-baggage
pellared Jul 24, 2024
4289316
Merge branch 'main' into utf8-baggage
XSAM Jul 27, 2024
66d8cd1
Add more comments
XSAM Aug 1, 2024
3060759
Merge branch 'main' into utf8-baggage
XSAM Aug 1, 2024
78c6cd7
Merge branch 'main' into utf8-baggage
XSAM Aug 2, 2024
59b314f
Merge branch 'main' into utf8-baggage
XSAM Aug 5, 2024
09371b8
Merge branch 'main' into utf8-baggage
XSAM Aug 8, 2024
9559edd
Merge branch 'main' into utf8-baggage
XSAM Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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)
- `NewMemberRaw` in `go.opentelemetry.io/otel/baggage` allows UTF-8 string. (#5132)

### Fixed

Expand Down
31 changes: 29 additions & 2 deletions baggage/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/url"
"strings"
"unicode/utf8"

"go.opentelemetry.io/otel/internal/baggage"
)
Expand Down Expand Up @@ -241,7 +242,12 @@ func NewMember(key, value string, props ...Property) (Member, error) {

// 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.
XSAM marked this conversation as resolved.
Show resolved Hide resolved
// However, the specific Propagators that are used to transmit baggage entries across
// component boundaries may impose their own restrictions on baggage key.
XSAM marked this conversation as resolved.
Show resolved Hide resolved
// 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,
Expand Down Expand Up @@ -313,9 +319,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()
}

Expand Down Expand Up @@ -630,6 +639,23 @@ func skipSpace(s string, offset int) int {
return i
}

// 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 {
return utf8.ValidString(s)
}

// validateKey checks if the string is a valid W3C Baggage key.
func validateKey(s string) bool {
if len(s) == 0 {
return false
Expand Down Expand Up @@ -658,6 +684,7 @@ func validateKeyChar(c int32) bool {
c == 0x7e
}

// validateValue checks if the string is a valid W3C Baggage value.
func validateValue(s string) bool {
for _, c := range s {
if !validateValueChar(c) {
Expand Down
48 changes: 48 additions & 0 deletions baggage/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ func TestNewKeyValueProperty(t *testing.T) {
assert.ErrorIs(t, err, errInvalidValue)
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)
Expand Down Expand Up @@ -409,6 +414,15 @@ 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"},
// The percent-encoded key won't be decoded.
"%C4%85%C5%9B%C4%87": {Value: "ąść"},
},
},
{
name: "invalid member: empty",
in: "foo=,,bar=",
Expand Down Expand Up @@ -861,6 +875,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())
}
Expand All @@ -882,6 +900,11 @@ func TestNewMember(t *testing.T) {
}
assert.Equal(t, expected, m)

// wrong value with invalid token
val = ";"
_, err = NewMember(key, val, p)
assert.ErrorIs(t, err, errInvalidValue)

// wrong value with wrong decoding
val = "%zzzzz"
_, err = NewMember(key, val, p)
Expand Down Expand Up @@ -926,6 +949,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)
Expand Down