-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add max baggage length as limitation #8222
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
base: main
Are you sure you want to change the base?
Changes from 13 commits
05a5c11
128f3c1
c274c8e
5eb9558
f3bcf1a
1bf7210
6feee46
4030ecd
f155676
64cf1f1
9ea953d
98da4b3
55e5b4f
285423e
35b72da
1c8caf8
832c87b
1424a1c
df9b633
0f14624
3bdee54
36456d1
d9d8cd2
965ba68
c782fa6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ package propagation // import "go.opentelemetry.io/otel/propagation" | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| "go.opentelemetry.io/otel/baggage" | ||
| "go.opentelemetry.io/otel/internal/errorhandler" | ||
|
|
@@ -15,7 +17,9 @@ const ( | |
|
|
||
| // W3C Baggage specification limits. | ||
| // https://www.w3.org/TR/baggage/#limits | ||
| maxMembers = 64 | ||
| maxMembers = 64 | ||
| maxBytesPerBaggageString = 8192 | ||
| maxParseErrors = 5 | ||
|
XSAM marked this conversation as resolved.
Outdated
|
||
| ) | ||
|
|
||
| // Baggage is a propagator that supports the W3C Baggage format. | ||
|
|
@@ -72,10 +76,36 @@ func extractMultiBaggage(parent context.Context, carrier ValuesGetter) context.C | |
| } | ||
|
|
||
| var members []baggage.Member | ||
| var totalBytes int | ||
| var parseErrors int | ||
| var truncateErr error | ||
| for _, bStr := range bVals { | ||
|
XSAM marked this conversation as resolved.
Outdated
|
||
| totalBytes += len(bStr) | ||
|
pellared marked this conversation as resolved.
|
||
| if totalBytes > maxBytesPerBaggageString { | ||
| parseErrors++ | ||
| if parseErrors <= maxParseErrors { | ||
|
Comment on lines
+88
to
+90
|
||
| truncateErr = errors.Join( | ||
| truncateErr, | ||
| fmt.Errorf( | ||
| "baggage: aggregate header size %d exceeds %d byte limit", | ||
| totalBytes, | ||
| maxBytesPerBaggageString, | ||
| ), | ||
| ) | ||
| } | ||
| break | ||
| } | ||
|
XSAM marked this conversation as resolved.
|
||
|
|
||
| currBag, err := baggage.Parse(bStr) | ||
| if err != nil { | ||
| errorhandler.GetErrorHandler().Handle(err) | ||
| if uw, ok := err.(interface{ Unwrap() []error }); ok { | ||
| parseErrors += len(uw.Unwrap()) | ||
| } else { | ||
| parseErrors++ | ||
| } | ||
|
XSAM marked this conversation as resolved.
|
||
| if parseErrors <= maxParseErrors { | ||
| truncateErr = errors.Join(truncateErr, err) | ||
| } | ||
|
XSAM marked this conversation as resolved.
Comment on lines
+105
to
+112
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If one header value returns a capped joined error from The existing propagation test covers many invalid headers, not one header with many invalid members. |
||
| } | ||
| if currBag.Len() == 0 { | ||
| continue | ||
|
|
@@ -86,10 +116,18 @@ func extractMultiBaggage(parent context.Context, carrier ValuesGetter) context.C | |
| } | ||
| } | ||
|
|
||
| if dropped := parseErrors - maxParseErrors; dropped > 0 { | ||
| truncateErr = errors.Join(truncateErr, fmt.Errorf("and %d more error(s)", dropped)) | ||
| } | ||
|
|
||
| b, err := baggage.New(members...) | ||
| if err != nil { | ||
| errorhandler.GetErrorHandler().Handle(err) | ||
| truncateErr = errors.Join(truncateErr, err) | ||
|
pellared marked this conversation as resolved.
|
||
| } | ||
| if truncateErr != nil { | ||
| errorhandler.GetErrorHandler().Handle(truncateErr) | ||
| } | ||
|
|
||
| if b.Len() == 0 { | ||
| return parent | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.