-
Notifications
You must be signed in to change notification settings - Fork 833
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
ImmutableBaggage.isKeyValid allows non-zero-length strings that contain nothing but spaces and/or tabs #2948
Comments
May I start working on this? |
@jkwatson I think this should be clarified in the specification because we want to have a consistent behavior across libraries |
Isn't key validity defined by the w3c spec already? |
https://www.w3.org/TR/baggage/#key "Leading and trailing whitespaces (OWS) are allowed but MUST be trimmed when converting the header into a data structure." If you trim leading and trailing whitespace on a string consisting of nothing but whitespace, you get an empty string. So !name.trim().isEmpty() is correct, and the current code is not correct. |
If we apply the w3c rules then we need to trim the keys always, for example " too" should overwrite "too". I want to clarify in the specs that we follow w3c rules which I think we don't say at the moment |
The proposed change in my first post in this bug report was to change !name.isEmpty() to !name.trim().isEmpty(), which always trims the keys. |
@bogdandrutu Can you shepherd a tweak to the specs so we can get #2950 merged? |
Has this issue been resolved? If not I would like to take a stab at it. |
I think we're still waiting on the specification issue to get resolved. |
any updates on the spec. ? |
Is it still open? private static boolean isKeyValid(String name) { // Check if the name is valid (non-empty after trimming) |
Is it still open? |
Someone will need to validate what the specification currently says about baggage keys and create a PR if we're not currently in compliance. |
As per this excerpt, the specification does not say anything about requirements for a key except for it being a string.
|
I created this PR. #6431 Please review it. |
It currently is:
private static boolean isKeyValid(String name) { return name != null && !name.isEmpty() && StringUtils.isPrintableString(name); }
It needs to be:
private static boolean isKeyValid(String name) { return name != null && !name.trim().isEmpty() && StringUtils.isPrintableString(name); }
Or it needs to use something like apache StringUtils isBlank.
The text was updated successfully, but these errors were encountered: