Skip to content

test: add another test case to confirm that issue 67 is fixed#12527

Closed
James-4u wants to merge 1 commit intogrpc:masterfrom
James-4u:james/issue-67
Closed

test: add another test case to confirm that issue 67 is fixed#12527
James-4u wants to merge 1 commit intogrpc:masterfrom
James-4u:james/issue-67

Conversation

@James-4u
Copy link
Copy Markdown

@James-4u James-4u commented Nov 21, 2025

This test verifies that hostname checking on :authority header works correctly
when multiple RPCs with different authority overrides are made on the same channel.

The test demonstrates:

  • Valid authorities matching the TLS certificate succeed
  • Invalid authorities are rejected with UNAVAILABLE status
  • Verification results are cached correctly (LRU cache)
  • Each authority is verified independently

I know #67 was fixed already, wanted to add a test case which ensures it's really solved and to close the issue as I thought it's not solved when I was looking at the repo issues.

Contribution by Gittensor, learn more at https://gittensor.io/

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Nov 21, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: SmartDever02 / name: aka James4u (7d9cc24)

@James-4u
Copy link
Copy Markdown
Author

@ejona86 requesting your review

@ejona86
Copy link
Copy Markdown
Member

ejona86 commented Nov 21, 2025

CC @kannanjgithub

Were the tests added in c28a7e3 insufficient? That issue should remain open until we flip GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK to default to true.

@James-4u
Copy link
Copy Markdown
Author

@ejona86 @kannanjgithub thanks for your reply.
btw can you plz let me know if this pr is worth to be merged?

@kannanjgithub
Copy link
Copy Markdown
Contributor

@SmartDever02 We do already have the unit tests that Eric mentioned in the above comment. Your PR creates a test that

  1. Tests using multiple good authorities instead of a single one, but it doesn't add any additional value over the original test that test a good authority and bad authority with the TrustManager separately.

  2. Mentions in the comments when the cache is used or not, but does no real assertions as to the number of times the TrustManager verification was invoked. There may be scope to track the counts in HostnameCheckingX509ExtendedTrustManager used by the existing tests and assert it in the tests.

@James-4u James-4u closed this Nov 24, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants