credentials/tls: Strip port before validating authority override#8726
credentials/tls: Strip port before validating authority override#8726easwars merged 7 commits intogrpc:masterfrom
Conversation
authority header
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8726 +/- ##
==========================================
+ Coverage 83.25% 83.37% +0.12%
==========================================
Files 419 418 -1
Lines 32427 32488 +61
==========================================
+ Hits 26997 27087 +90
+ Misses 4047 4017 -30
- Partials 1383 1384 +1
🚀 New features to boost your workflow:
|
authority headerauthority header
There was a problem hiding this comment.
The existing tests for authority override are located here:
Could you please add a new test case to the list of successful cases? For the invalid override tests, it would be great if you could refactor them into a table-driven test (similar to the success cases) and include a case for the changes in this PR. This will help avoid code duplication.
There was a problem hiding this comment.
Thanks for the review! Same has been done.
|
This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed. |
arjan-bal
left a comment
There was a problem hiding this comment.
LGTM, adding a second reviewer.
authority header
Fixes: #8719
Splits the host and port in the HTTP2
:authorityheader inValidateAuthoritybefore callingVerifyHostname. Added test cases to check multiple different types of values possible for:authorityRELEASE NOTES: