Skip to content

Commit e499ca8

Browse files
MrAliasMrAlias
authored
Fix validation for tracestate with vendor and add tests (#1581)
* Fix validation for tracestate with vendor * Add changes to changelog
1 parent 43886e5 commit e499ca8

File tree

3 files changed

+249
-5
lines changed

3 files changed

+249
-5
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
4949
- The sequential timing check of timestamps in the stdout exporter are now setup explicitly to be sequential (#1571). (#1572)
5050
- Windows build of Jaeger tests now compiles with OS specific functions (#1576). (#1577)
5151
- The sequential timing check of timestamps of go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue are now setup explicitly to be sequential (#1578). (#1579)
52+
- Validate tracestate header keys with vedors according to the W3C TraceContext specification (#1475). (#1581)
5253

5354
## [0.17.0] - 2020-02-12
5455

‎trace/trace.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ const (
4848

4949
// based on the W3C Trace Context specification, see https://www.w3.org/TR/trace-context-1/#tracestate-header
5050
traceStateKeyFormat = `[a-z][_0-9a-z\-\*\/]{0,255}`
51-
traceStateKeyFormatWithMultiTenantVendor = `[a-z][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}`
51+
traceStateKeyFormatWithMultiTenantVendor = `[a-z0-9][_0-9a-z\-\*\/]{0,240}@[a-z][_0-9a-z\-\*\/]{0,13}`
5252
traceStateValueFormat = `[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]`
5353

5454
traceStateMaxListMembers = 32

‎trace/trace_test.go

Lines changed: 247 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -699,14 +699,23 @@ func TestTraceStateFromKeyValues(t *testing.T) {
699699
expectedErr: errInvalidTraceStateMembersNumber,
700700
},
701701
{
702-
name: "Duplicate",
702+
name: "Duplicate key",
703703
kvs: []attribute.KeyValue{
704704
attribute.String("key1", "val1"),
705705
attribute.String("key1", "val2"),
706706
},
707707
expectedTraceState: TraceState{},
708708
expectedErr: errInvalidTraceStateDuplicate,
709709
},
710+
{
711+
name: "Duplicate key/value",
712+
kvs: []attribute.KeyValue{
713+
attribute.String("key1", "val1"),
714+
attribute.String("key1", "val1"),
715+
},
716+
expectedTraceState: TraceState{},
717+
expectedErr: errInvalidTraceStateDuplicate,
718+
},
710719
{
711720
name: "Invalid key/value",
712721
kvs: []attribute.KeyValue{
@@ -715,23 +724,257 @@ func TestTraceStateFromKeyValues(t *testing.T) {
715724
expectedTraceState: TraceState{},
716725
expectedErr: errInvalidTraceStateKeyValue,
717726
},
727+
{
728+
name: "Full character set",
729+
kvs: []attribute.KeyValue{
730+
attribute.String(
731+
"abcdefghijklmnopqrstuvwxyz0123456789_-*/",
732+
" !\"#$%&'()*+-./0123456789:;<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~",
733+
),
734+
},
735+
expectedTraceState: TraceState{[]attribute.KeyValue{
736+
attribute.String(
737+
"abcdefghijklmnopqrstuvwxyz0123456789_-*/",
738+
" !\"#$%&'()*+-./0123456789:;<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~",
739+
),
740+
}},
741+
},
742+
{
743+
name: "Full character set with vendor",
744+
kvs: []attribute.KeyValue{
745+
attribute.String(
746+
"abcdefghijklmnopqrstuvwxyz0123456789_-*/@a-z0-9_-*/",
747+
"!\"#$%&'()*+-./0123456789:;<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~",
748+
),
749+
},
750+
expectedTraceState: TraceState{[]attribute.KeyValue{
751+
attribute.String(
752+
"abcdefghijklmnopqrstuvwxyz0123456789_-*/@a-z0-9_-*/",
753+
"!\"#$%&'()*+-./0123456789:;<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~",
754+
),
755+
}},
756+
},
757+
{
758+
name: "Full character with vendor starting with number",
759+
kvs: []attribute.KeyValue{
760+
attribute.String(
761+
"0123456789_-*/abcdefghijklmnopqrstuvwxyz@a-z0-9_-*/",
762+
"!\"#$%&'()*+-./0123456789:;<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~",
763+
),
764+
},
765+
expectedTraceState: TraceState{[]attribute.KeyValue{
766+
attribute.String(
767+
"0123456789_-*/abcdefghijklmnopqrstuvwxyz@a-z0-9_-*/",
768+
"!\"#$%&'()*+-./0123456789:;<>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~",
769+
),
770+
}},
771+
},
772+
{
773+
name: "One field",
774+
kvs: []attribute.KeyValue{
775+
attribute.String("foo", "1"),
776+
},
777+
expectedTraceState: TraceState{[]attribute.KeyValue{
778+
attribute.String("foo", "1"),
779+
}},
780+
},
781+
{
782+
name: "Two fields",
783+
kvs: []attribute.KeyValue{
784+
attribute.String("foo", "1"),
785+
attribute.String("bar", "2"),
786+
},
787+
expectedTraceState: TraceState{[]attribute.KeyValue{
788+
attribute.String("foo", "1"),
789+
attribute.String("bar", "2"),
790+
}},
791+
},
792+
{
793+
name: "Long field key",
794+
kvs: []attribute.KeyValue{
795+
attribute.String("foo", "1"),
796+
attribute.String(
797+
"zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz",
798+
"1",
799+
),
800+
},
801+
expectedTraceState: TraceState{[]attribute.KeyValue{
802+
attribute.String("foo", "1"),
803+
attribute.String(
804+
"zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz",
805+
"1",
806+
),
807+
}},
808+
},
809+
{
810+
name: "Long field key with vendor",
811+
kvs: []attribute.KeyValue{
812+
attribute.String("foo", "1"),
813+
attribute.String(
814+
"ttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt@vvvvvvvvvvvvvv",
815+
"1",
816+
),
817+
},
818+
expectedTraceState: TraceState{[]attribute.KeyValue{
819+
attribute.String("foo", "1"),
820+
attribute.String(
821+
"ttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt@vvvvvvvvvvvvvv",
822+
"1",
823+
),
824+
}},
825+
},
826+
{
827+
name: "Invalid whitespace value",
828+
kvs: []attribute.KeyValue{
829+
attribute.String("foo", "1 \t "),
830+
},
831+
expectedTraceState: TraceState{},
832+
expectedErr: errInvalidTraceStateKeyValue,
833+
},
834+
{
835+
name: "Invalid whitespace key",
836+
kvs: []attribute.KeyValue{
837+
attribute.String(" \t bar", "2"),
838+
},
839+
expectedTraceState: TraceState{},
840+
expectedErr: errInvalidTraceStateKeyValue,
841+
},
842+
{
843+
name: "Empty header value",
844+
kvs: []attribute.KeyValue{
845+
attribute.String("", ""),
846+
},
847+
expectedTraceState: TraceState{},
848+
expectedErr: errInvalidTraceStateKeyValue,
849+
},
850+
{
851+
name: "Space in key",
852+
kvs: []attribute.KeyValue{
853+
attribute.String("foo ", "1"),
854+
},
855+
expectedTraceState: TraceState{},
856+
expectedErr: errInvalidTraceStateKeyValue,
857+
},
858+
{
859+
name: "Capitalized key",
860+
kvs: []attribute.KeyValue{
861+
attribute.String("FOO", "1"),
862+
},
863+
expectedTraceState: TraceState{},
864+
expectedErr: errInvalidTraceStateKeyValue,
865+
},
866+
{
867+
name: "Period in key",
868+
kvs: []attribute.KeyValue{
869+
attribute.String("foo.bar", "1"),
870+
},
871+
expectedTraceState: TraceState{},
872+
expectedErr: errInvalidTraceStateKeyValue,
873+
},
874+
{
875+
name: "Empty vendor",
876+
kvs: []attribute.KeyValue{
877+
attribute.String("foo@", "1"),
878+
attribute.String("bar", "2"),
879+
},
880+
expectedTraceState: TraceState{},
881+
expectedErr: errInvalidTraceStateKeyValue,
882+
},
883+
{
884+
name: "Empty key for vendor",
885+
kvs: []attribute.KeyValue{
886+
attribute.String("@foo", "1"),
887+
attribute.String("bar", "2"),
888+
},
889+
expectedTraceState: TraceState{},
890+
expectedErr: errInvalidTraceStateKeyValue,
891+
},
892+
{
893+
name: "Double @",
894+
kvs: []attribute.KeyValue{
895+
attribute.String("foo@@bar", "1"),
896+
attribute.String("bar", "2"),
897+
},
898+
expectedTraceState: TraceState{},
899+
expectedErr: errInvalidTraceStateKeyValue,
900+
},
901+
{
902+
name: "Compound vendor",
903+
kvs: []attribute.KeyValue{
904+
attribute.String("foo@bar@baz", "1"),
905+
attribute.String("bar", "2"),
906+
},
907+
expectedTraceState: TraceState{},
908+
expectedErr: errInvalidTraceStateKeyValue,
909+
},
910+
{
911+
name: "Key too long",
912+
kvs: []attribute.KeyValue{
913+
attribute.String("foo", "1"),
914+
attribute.String("zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", "1"),
915+
},
916+
expectedTraceState: TraceState{},
917+
expectedErr: errInvalidTraceStateKeyValue,
918+
},
919+
{
920+
name: "Key too long with vendor",
921+
kvs: []attribute.KeyValue{
922+
attribute.String("foo", "1"),
923+
attribute.String("tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt@v", "1"),
924+
},
925+
expectedTraceState: TraceState{},
926+
expectedErr: errInvalidTraceStateKeyValue,
927+
},
928+
{
929+
name: "Vendor too long",
930+
kvs: []attribute.KeyValue{
931+
attribute.String("foo", "1"),
932+
attribute.String("t@vvvvvvvvvvvvvvv", "1"),
933+
},
934+
expectedTraceState: TraceState{},
935+
expectedErr: errInvalidTraceStateKeyValue,
936+
},
937+
{
938+
name: "Equal sign in value",
939+
kvs: []attribute.KeyValue{
940+
attribute.String("foo", "bar=baz"),
941+
},
942+
expectedTraceState: TraceState{},
943+
expectedErr: errInvalidTraceStateKeyValue,
944+
},
945+
{
946+
name: "Empty value",
947+
kvs: []attribute.KeyValue{
948+
attribute.String("foo", ""),
949+
attribute.String("bar", "3"),
950+
},
951+
expectedTraceState: TraceState{},
952+
expectedErr: errInvalidTraceStateKeyValue,
953+
},
954+
}
955+
956+
messageFunc := func(kvs []attribute.KeyValue) []string {
957+
var out []string
958+
for _, kv := range kvs {
959+
out = append(out, fmt.Sprintf("%s=%s", kv.Key, kv.Value.AsString()))
960+
}
961+
return out
718962
}
719963

720964
for _, tc := range testCases {
721965
t.Run(tc.name, func(t *testing.T) {
722966
result, err := TraceStateFromKeyValues(tc.kvs...)
723967
if tc.expectedErr != nil {
724-
require.Error(t, err)
968+
require.Error(t, err, messageFunc(tc.kvs))
725969
assert.Equal(t, TraceState{}, result)
726970
assert.Equal(t, tc.expectedErr, err)
727971
} else {
728-
require.NoError(t, err)
972+
require.NoError(t, err, messageFunc(tc.kvs))
729973
assert.NotNil(t, tc.expectedTraceState)
730974
assert.Equal(t, tc.expectedTraceState, result)
731975
}
732976
})
733977
}
734-
735978
}
736979

737980
func assertSpanContextEqual(got SpanContext, want SpanContext) bool {

0 commit comments

Comments
 (0)