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

Inconsistence of baggage value restriction #3144

Closed
leizhag opened this issue Sep 6, 2022 · 2 comments · Fixed by #3226
Closed

Inconsistence of baggage value restriction #3144

leizhag opened this issue Sep 6, 2022 · 2 comments · Fixed by #3226
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@leizhag
Copy link

leizhag commented Sep 6, 2022

Description

Member.String() accpets UTF-8 strings as baggage member values, and URL-encodes them. But it seems this URL-encoding has no effect since baggage.NewMember() does not accept UTF-8 strings.

Environment

  • opentelemetry-go version: 1.9.0

Steps To Reproduce

  1. baggage.NewMember("", ";") returns an error.

Expected behavior

baggage.NewMember("", ";") does not return errors or Member.String() does no URL-encoding.

@leizhag leizhag added the bug Something isn't working label Sep 6, 2022
@MrAlias
Copy link
Contributor

MrAlias commented Sep 6, 2022

baggage.NewMember("", ";") contains an invalid key: https://go.dev/play/p/Xeoi2wArkIn

It also contains an invalid value according to the W3C baggage specification: https://go.dev/play/p/oGeHN3Y52wl

The error returned from NewMember is valid based on how that function is defined:

// NewMember returns a new Member from the passed arguments. An error is
// returned if the created Member would be invalid according to the W3C
// Baggage specification.

As for underlying issue, the accepted value of NewMember is not decoded as the specification has added:

A value contains a string whose character encoding MUST be UTF-8 [Encoding]. Any characters outside of the baggage-octet range of characters MUST be percent-encoded. Characters which are not required to be percent-encoded MAY be percent-encoded. Percent-encoding is defined in [RFC3986], Section 2.1: https://datatracker.ietf.org/doc/html/rfc3986#section-2.1.

When decoding the value, percent-encoded octet sequences that do not match the UTF-8 encoding scheme MUST be replaced with the replacement character (U+FFFD).

baggage.NewMember("key", "%3B") should be interpreted with a value of ;. Instead it is interpreted directly: https://go.dev/play/p/16d_eij-CWt

The value accepted by NewMember needs to be decoded.

@MrAlias MrAlias added the help wanted Extra attention is needed label Sep 6, 2022
@NewReStarter
Copy link
Contributor

@MrAlias hey, can i take this bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
Archived in project
4 participants