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 29 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
See our [versioning policy](VERSIONING.md) for more information about these stability guarantees. (#5629)
- Add `InstrumentationScope` field to `SpanStub` in `go.opentelemetry.io/otel/sdk/trace/tracetest`, as a replacement for the deprecated `InstrumentationLibrary`. (#5627)

### Changed

- `NewMemberRaw`, `NewKeyProperty` and `NewKeyValuePropertyRaw` in `go.opentelemetry.io/otel/baggage` allow UTF-8 string in key. (#5132)
pellared marked this conversation as resolved.
Show resolved Hide resolved

### Fixed

- Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their corresponding environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5584)
Expand Down
94 changes: 80 additions & 14 deletions baggage/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ type Property struct {

// NewKeyProperty returns a new Property for key.
//
// The passed key must be valid, non-empty UTF-8 string.
// 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)
}

Expand All @@ -62,6 +63,10 @@ func NewKeyProperty(key string) (Property, error) {
// Notice: Consider using [NewKeyValuePropertyRaw] instead
// that does not require percent-encoding of the value.
func NewKeyValueProperty(key, value string) (Property, error) {
if !validateKey(key) {
pellared marked this conversation as resolved.
Show resolved Hide resolved
return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key)
}

if !validateValue(value) {
return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value)
}
Expand All @@ -74,11 +79,20 @@ func NewKeyValueProperty(key, value string) (Property, error) {

// NewKeyValuePropertyRaw returns a new Property for key with value.
//
// The passed key must be compliant with W3C Baggage specification.
// The passed key must be valid, non-empty UTF-8 string.
// The passed 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.
XSAM marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand Down Expand Up @@ -115,12 +129,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
}

Expand All @@ -139,6 +156,11 @@ func (p Property) Value() (string, bool) {
// String encodes Property into a header string compliant with the W3C Baggage
// specification.
func (p Property) String() string {
// W3C Baggage specification does not allow percent-encoded keys.
if !validateKey(p.key) {
return ""
pellared marked this conversation as resolved.
Show resolved Hide resolved
}

if p.hasValue {
return fmt.Sprintf("%s%s%v", p.key, keyValueDelimiter, valueEscape(p.value))
}
Expand Down Expand Up @@ -203,9 +225,14 @@ func (p properties) validate() error {
// String encodes properties into a header string compliant with the W3C Baggage
// specification.
func (p properties) String() string {
props := make([]string, len(p))
for i, prop := range p {
props[i] = prop.String()
props := make([]string, 0, len(p))
for _, prop := range p {
s := prop.String()

// Ignored empty properties.
if s != "" {
props = append(props, s)
}
}
return strings.Join(props, propertyDelimiter)
}
Expand All @@ -230,6 +257,10 @@ type Member struct {
// 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) {
pellared marked this conversation as resolved.
Show resolved Hide resolved
return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key)
}

if !validateValue(value) {
return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value)
}
Expand All @@ -242,7 +273,13 @@ 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 must be valid, non-empty UTF-8 string.
// The passed 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.
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 @@ -340,9 +377,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 All @@ -358,9 +398,11 @@ 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.
// W3C Baggage specification does not allow percent-encoded keys.
if !validateKey(m.key) {
return ""
}
pellared marked this conversation as resolved.
Show resolved Hide resolved

s := m.key + keyValueDelimiter + valueEscape(m.value)
if len(m.properties) > 0 {
s += propertyDelimiter + m.properties.String()
Expand Down Expand Up @@ -557,11 +599,16 @@ func (b Baggage) Len() int {
func (b Baggage) String() string {
members := make([]string, 0, len(b.list))
for k, v := range b.list {
members = append(members, Member{
s := Member{
key: k,
value: v.Value,
properties: fromInternalProperties(v.Properties),
}.String())
}.String()

// Ignored empty members.
if s != "" {
members = append(members, s)
}
}
return strings.Join(members, listDelimiter)
}
Expand Down Expand Up @@ -748,6 +795,24 @@ var safeKeyCharset = [utf8.RuneSelf]bool{
'~': true,
}

// validateBaggageName checks if the string is a valid OpenTelemetry Baggage name.
// Baggage name is a valid, non-empty 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.
// Empty string is also a valid UTF-8 string.
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 All @@ -766,6 +831,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) {
Expand Down
Loading
Loading