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

Clarify the allowance of an empty string in the baggage names #4082

Closed
XSAM opened this issue Jun 19, 2024 · 2 comments · Fixed by #4144
Closed

Clarify the allowance of an empty string in the baggage names #4082

XSAM opened this issue Jun 19, 2024 · 2 comments · Fixed by #4144
Labels
spec:baggage Related to the specification/baggage directory triage:deciding:tc-inbox Needs attention from the TC in order to move forward

Comments

@XSAM
Copy link
Member

XSAM commented Jun 19, 2024

What are you trying to achieve?

Clarify the allowance of an empty string in the baggage names.

Additional context.

After #3801, we allow the UTF-8 string in the baggage. However, an empty string is also a valid UTF-8 string.

UTF8-octets = *( UTF8-char )

https://datatracker.ietf.org/doc/html/rfc3629

This means we allow our baggage names to be empty. = and =bar become valid baggages, which are strange baggage.


FYI, the W3C baggage specification does not allow an empty baggage name.

key = token ; as defined in RFC 7230, Section 3.2.6

https://www.w3.org/TR/baggage/#definition


I am not against the design of allowing empty strings in baggage names, as it does not bring compatible issues while parsing W3C baggage. But it might be worth mentioning in the doc that we actually allow empty strings in baggage names, so it won't confuse people while implementing.

Related PR: open-telemetry/opentelemetry-go#5132

@XSAM XSAM added the spec:baggage Related to the specification/baggage directory label Jun 19, 2024
@XSAM XSAM changed the title Clarify the empty string in baggage names Clarify the allowance of an empty string in the baggage names Jun 20, 2024
@svrnm svrnm added the triage:deciding:tc-inbox Needs attention from the TC in order to move forward label Jul 8, 2024
@svrnm
Copy link
Member

svrnm commented Jul 8, 2024

@open-telemetry/technical-committee please take a look! This might have been an oversight while #3801 was created and should be easy to fix, but take a look and let @XSAM know what to do.

@yurishkuro
Copy link
Member

sgtm to me to restrict names to non-empty strings. Empty values should ok imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:baggage Related to the specification/baggage directory triage:deciding:tc-inbox Needs attention from the TC in order to move forward
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants