w3c / baggage

Propagation format for distributed context: Baggage
https://w3c.github.io/baggage/
Other
48 stars 18 forks source link

Document rationale for 255 leading spaces in tracestate values or remove it #23

Closed codefromthecrypt closed 4 years ago

codefromthecrypt commented 4 years ago

@reyang changed the spec to allow 255 leading space characters and @SergeyKanzhelev merged it the next day. There was zero rationale for this, yet it is very unintuitive

https://github.com/w3c/trace-context/pull/135

As the intuitive behavior was reversed, we need an explanation. This makes for very strange parsers especially odd when so much thrash happened in the name of making parsing easier in the past. This has caused a couple people to scratch their heads and it is unacceptable for a change like this to have been merged in the first place.

I raised a comment there. but it was ignored, so making this an issue instead. Here's the comment:


trying to understand why this is allowing 255 spaces before a non-space.

188 has a lot of talk about impact of presumably having a trailing empty member. I saw no code references, just talk about test cases invisible to open source, yet used to justify that change. Regardless, this literally permits 255 empty spaces than a non-space. If what's said in 188 is true, and that libraries arbitrarily add white (please cite exact libraries and versions).. what does this help?

I think it just makes sure space is only allowed for left-padding a non-space. If correct, the description should be updated. If incorrect, I guess I am misreading the abnf and would appreciate knowing how.

codefromthecrypt commented 4 years ago

raised in wrong repo https://github.com/w3c/trace-context/issues/409