Auto-port 4.1: Route synchronous onLookupComplete exceptions via fireExceptionCaught#16824
Merged
Conversation
…#16794) Motivation: Exceptions thrown synchronously by the user-supplied onLookupComplete callback in AbstractSniHandler subclasses are caught by SslClientHelloHandler.decode()'s outer catch (Exception) and turned into a select(ctx, null) retry, falling back to the default SslContext. The user-callback exception never reaches the pipeline. The async branch of the same select() method already routes such exceptions via ctx.fireExceptionCaught(...). The sync branch lacks this routing. Modification: - Add an inner try/catch around the synchronous onLookupComplete(...) call in SslClientHelloHandler.select(), mirroring the routing the async FutureListener already performs. - Add SslClientHelloHandlerTest covering the sync path. - Update SniHandlerTest.testNonAsciiServerNameParsing to reflect the new routing behaviour (the exception is now fired on the pipeline rather than thrown directly from writeInbound). Result: User-callback exceptions are routed via fireExceptionCaught regardless of whether the lookup Future is completed synchronously or asynchronously. The silent select(ctx, null) retry path is no longer triggered by user-callback exceptions. Fixes #16790 Signed-off-by: Daeho Kwon <trewq231@naver.com> (cherry picked from commit 4ce9f17)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-port of #16794 to 4.1
Cherry-picked commit: 4ce9f17
Motivation:
Exceptions thrown synchronously by the user-supplied onLookupComplete
callback in AbstractSniHandler subclasses are caught by
SslClientHelloHandler.decode()'s outer catch (Exception) and turned
into a select(ctx, null) retry, falling back to the default
SslContext. The user-callback exception never reaches the pipeline.
The async branch of the same select() method already routes such
exceptions via ctx.fireExceptionCaught(...). The sync branch lacks
this routing.
Modification:
Result:
User-callback exceptions are routed via fireExceptionCaught regardless
of whether the lookup Future is completed synchronously or
asynchronously. The silent select(ctx, null) retry path is no longer
triggered by user-callback exceptions.
Fixes #16790