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

C#: metadata.Get and GetAll should accept uppercase keys #27383

Merged

Conversation

jtattermusch
Copy link
Contributor

Fixes #26551.

Similar to #26552, but the implementation is cleaner and it adds tests.

Note that this is mostly a cosmetic change, since according to the gRPC spec, the metadata keys are supposed to be lowercase. But at this point, creating a new Metadata.Entry("KEY", "value") does lowercasing of the key under the hood,
and a subsequent metadata.Get("KEY") won't find the entry. (it's basically a cornercase since there's no real reason to use uppercase metadata keys with gRPC when the spec only allows lowercase.

@jtattermusch
Copy link
Contributor Author

@StingyJack feel free to also review.

@jtattermusch
Copy link
Contributor Author

@jtattermusch jtattermusch merged commit d64f75d into grpc:master Sep 18, 2021
@StingyJack
Copy link

Its not only uppercase,

TransactionScopeId
CorrelationId
UserActivitySnowflake
etc...

A separate function to wrap a string.Equals is overkill. There is no difference between these changes and the ones I submitted on the other PR when yours just wraps the same string.Equals in a function that looks odd to invoke and that makes use of this.key and key to compare. You think that this is clearer? Seems like you got some pride in your eyes thats affecting your vision.

@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Sep 20, 2021
lidizheng pushed a commit to lidizheng/grpc that referenced this pull request Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported Specifies if the PR has been imported to the internal repository lang/C# release notes: yes Indicates if PR needs to be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to retrieve ServerCallContext.RequestHeader values using the Key they were added with
3 participants