-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(core): Keep all property values in baggage header #17847
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
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for fixing this!
Could you add or adjust one of the browser integration tests as well? I believe we already have a test that shows how we deal with 3rd party baggage values but it doesn't include the more complex entries with properties.
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
https://www.w3.org/TR/baggage/#example contains an example where properties can include an
=
, e.g.key3=value3; propertyKey=propertyValue
This PR updates our baggage parsing to not swallow these property values
closes #17845