diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ec88f882c5..a2376a26e50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Improve `go.opentelemetry.io/otel/trace.TraceState`'s performance. (#4722) - Improve `go.opentelemetry.io/otel/propagation.TraceContext`'s performance. (#4721) +- Improve `go.opentelemetry.io/otel/baggage` performance. (#4743) ## [1.21.0/0.44.0] 2023-11-16 diff --git a/baggage/baggage.go b/baggage/baggage.go index 84532cb1da3..c1f06fab18c 100644 --- a/baggage/baggage.go +++ b/baggage/baggage.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "net/url" - "regexp" "strings" "go.opentelemetry.io/otel/internal/baggage" @@ -32,16 +31,6 @@ const ( listDelimiter = "," keyValueDelimiter = "=" propertyDelimiter = ";" - - keyDef = `([\x21\x23-\x27\x2A\x2B\x2D\x2E\x30-\x39\x41-\x5a\x5e-\x7a\x7c\x7e]+)` - valueDef = `([\x21\x23-\x2b\x2d-\x3a\x3c-\x5B\x5D-\x7e]*)` - keyValueDef = `\s*` + keyDef + `\s*` + keyValueDelimiter + `\s*` + valueDef + `\s*` -) - -var ( - keyRe = regexp.MustCompile(`^` + keyDef + `$`) - valueRe = regexp.MustCompile(`^` + valueDef + `$`) - propertyRe = regexp.MustCompile(`^(?:\s*` + keyDef + `\s*|` + keyValueDef + `)$`) ) var ( @@ -67,7 +56,7 @@ type Property struct { // // If key is invalid, an error will be returned. func NewKeyProperty(key string) (Property, error) { - if !keyRe.MatchString(key) { + if !validateKey(key) { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) } @@ -79,10 +68,10 @@ func NewKeyProperty(key string) (Property, error) { // // If key or value are invalid, an error will be returned. func NewKeyValueProperty(key, value string) (Property, error) { - if !keyRe.MatchString(key) { + if !validateKey(key) { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) } - if !valueRe.MatchString(value) { + if !validateValue(value) { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value) } @@ -106,20 +95,11 @@ func parseProperty(property string) (Property, error) { return newInvalidProperty(), nil } - match := propertyRe.FindStringSubmatch(property) - if len(match) != 4 { + p, ok := parsePropertyInternal(property) + if !ok { return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidProperty, property) } - var p Property - if match[1] != "" { - p.key = match[1] - } else { - p.key = match[2] - p.value = match[3] - p.hasValue = true - } - return p, nil } @@ -130,10 +110,10 @@ func (p Property) validate() error { return fmt.Errorf("invalid property: %w", err) } - if !keyRe.MatchString(p.key) { + if !validateKey(p.key) { return errFunc(fmt.Errorf("%w: %q", errInvalidKey, p.key)) } - if p.hasValue && !valueRe.MatchString(p.value) { + if p.hasValue && !validateValue(p.value) { return errFunc(fmt.Errorf("%w: %q", errInvalidValue, p.value)) } if !p.hasValue && p.value != "" { @@ -305,10 +285,10 @@ func parseMember(member string) (Member, error) { if err != nil { return newInvalidMember(), fmt.Errorf("%w: %q", err, value) } - if !keyRe.MatchString(key) { + if !validateKey(key) { return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key) } - if !valueRe.MatchString(value) { + if !validateValue(value) { return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value) } @@ -323,10 +303,10 @@ func (m Member) validate() error { return fmt.Errorf("%w: %q", errInvalidMember, m) } - if !keyRe.MatchString(m.key) { + if !validateKey(m.key) { return fmt.Errorf("%w: %q", errInvalidKey, m.key) } - if !valueRe.MatchString(m.value) { + if !validateValue(m.value) { return fmt.Errorf("%w: %q", errInvalidValue, m.value) } return m.properties.validate() @@ -550,3 +530,133 @@ func (b Baggage) String() string { } return strings.Join(members, listDelimiter) } + +// parsePropertyInternal attempts to decode a Property from the passed string. +// It follows the spec at https://www.w3.org/TR/baggage/#definition. +func parsePropertyInternal(s string) (p Property, ok bool) { + // For the entire function we will use " key = value " as an example. + // Attempting to parse the key. + // First skip spaces at the beginning "< >key = value " (they could be empty). + index := skipSpace(s, 0) + + // Parse the key: " = value ". + keyStart := index + keyEnd := index + for _, c := range s[keyStart:] { + if !validateKeyChar(c) { + break + } + keyEnd++ + } + + // If we couldn't find any valid key character, + // it means the key is either empty or invalid. + if keyStart == keyEnd { + return + } + + // Skip spaces after the key: " key< >= value ". + index = skipSpace(s, keyEnd) + + if index == len(s) { + // A key can have no value, like: " key ". + ok = true + p.key = s[keyStart:keyEnd] + return + } + + // If we have not reached the end and we can't find the '=' delimiter, + // it means the property is invalid. + if s[index] != keyValueDelimiter[0] { + return + } + + // Attempting to parse the value. + // Match: " key =< >value ". + index = skipSpace(s, index+1) + + // Match the value string: " key = ". + // A valid property can be: " key =". + // Therefore, we don't have to check if the value is empty. + valueStart := index + valueEnd := index + for _, c := range s[valueStart:] { + if !validateValueChar(c) { + break + } + valueEnd++ + } + + // Skip all trailing whitespaces: " key = value< >". + index = skipSpace(s, valueEnd) + + // If after looking for the value and skipping whitespaces + // we have not reached the end, it means the property is + // invalid, something like: " key = value value1". + if index != len(s) { + return + } + + ok = true + p.key = s[keyStart:keyEnd] + p.hasValue = true + p.value = s[valueStart:valueEnd] + return +} + +func skipSpace(s string, offset int) int { + i := offset + for ; i < len(s); i++ { + c := s[i] + if c != ' ' && c != '\t' { + break + } + } + return i +} + +func validateKey(s string) bool { + if len(s) == 0 { + return false + } + + for _, c := range s { + if !validateKeyChar(c) { + return false + } + } + + return true +} + +func validateKeyChar(c int32) bool { + return (c >= 0x23 && c <= 0x27) || + (c >= 0x30 && c <= 0x39) || + (c >= 0x41 && c <= 0x5a) || + (c >= 0x5e && c <= 0x7a) || + c == 0x21 || + c == 0x2a || + c == 0x2b || + c == 0x2d || + c == 0x2e || + c == 0x7c || + c == 0x7e +} + +func validateValue(s string) bool { + for _, c := range s { + if !validateValueChar(c) { + return false + } + } + + return true +} + +func validateValueChar(c int32) bool { + return (c >= 0x23 && c <= 0x2b) || + (c >= 0x2d && c <= 0x3a) || + (c >= 0x3c && c <= 0x5b) || + (c >= 0x5d && c <= 0x7e) || + c == 0x21 +} diff --git a/baggage/baggage_test.go b/baggage/baggage_test.go index 4bac6707ea0..04395efbca8 100644 --- a/baggage/baggage_test.go +++ b/baggage/baggage_test.go @@ -33,7 +33,7 @@ func init() { rng = rand.New(rand.NewSource(1)) } -func TestKeyRegExp(t *testing.T) { +func TestValidateKeyChar(t *testing.T) { // ASCII only invalidKeyRune := []rune{ '\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', @@ -45,11 +45,11 @@ func TestKeyRegExp(t *testing.T) { } for _, ch := range invalidKeyRune { - assert.NotRegexp(t, keyDef, fmt.Sprintf("%c", ch)) + assert.False(t, validateKeyChar(ch)) } } -func TestValueRegExp(t *testing.T) { +func TestValidateValueChar(t *testing.T) { // ASCII only invalidValueRune := []rune{ '\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', @@ -60,7 +60,7 @@ func TestValueRegExp(t *testing.T) { } for _, ch := range invalidValueRune { - assert.NotRegexp(t, `^`+valueDef+`$`, fmt.Sprintf("invalid-%c-value", ch)) + assert.False(t, validateValueChar(ch)) } } @@ -415,10 +415,15 @@ func TestBaggageParse(t *testing.T) { err: errInvalidValue, }, { - name: "invalid property: invalid key", + name: "invalid property: no key", in: "foo=1;=v", err: errInvalidProperty, }, + { + name: "invalid property: invalid key", + in: "foo=1;key\\=v", + err: errInvalidProperty, + }, { name: "invalid property: invalid value", in: "foo=1;key=\\",