From bff62f4805e8484dc611f067388a6a3751eb0388 Mon Sep 17 00:00:00 2001 From: Cris Liao Date: Wed, 20 May 2026 17:24:27 -0700 Subject: [PATCH] Fix NPE in NettyResponseChannel when root cause exception has null message CompletableFuture.orTimeout() creates a TimeoutException with no message. Utils.getRootCause() returns that exception, and calling getMessage() on it returns null, causing an NPE at getErrorResponse() line 593. Add null check before using the root cause message to set FAILURE_REASON_HEADER. Co-Authored-By: Claude Sonnet 4.6 --- .../ambry/rest/NettyResponseChannel.java | 9 ++++-- .../ambry/rest/NettyResponseChannelTest.java | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java b/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java index c6d41bad63..be118b8419 100644 --- a/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java +++ b/ambry-rest/src/main/java/com/github/ambry/rest/NettyResponseChannel.java @@ -589,9 +589,12 @@ private FullHttpResponse getErrorResponse(Throwable cause) { errorResponseStatus = ResponseStatus.getResponseStatus(restServiceErrorCode); status = getHttpResponseStatus(errorResponseStatus); if (shouldSendFailureReason(status, restServiceException)) { - errReason = new String( - Utils.getRootCause(cause).getMessage().replaceAll("[\n\t\r]", " ").getBytes(StandardCharsets.US_ASCII), - StandardCharsets.US_ASCII); + String rootMessage = Utils.getRootCause(cause).getMessage(); + if (rootMessage != null) { + errReason = new String( + rootMessage.replaceAll("[\n\t\r]", " ").getBytes(StandardCharsets.US_ASCII), + StandardCharsets.US_ASCII); + } } if (restServiceException.shouldIncludeExceptionMetadataInResponse()) { errHeaders = restServiceException.getExceptionHeadersMap(); diff --git a/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java b/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java index 24c42699d1..a2fe47d958 100644 --- a/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java +++ b/ambry-rest/src/test/java/com/github/ambry/rest/NettyResponseChannelTest.java @@ -462,6 +462,27 @@ public void setFailureReasonInResponseTest() throws Exception { assertFalse("Channel not closed on the server", channel.isActive()); } + /** + * Tests that no NPE occurs when the root cause of a RestServiceException has a null message (e.g., + * {@link java.util.concurrent.TimeoutException} created by {@link java.util.concurrent.CompletableFuture#orTimeout}). + */ + @Test + public void setFailureReasonNullMessageNoNpeTest() throws Exception { + HttpRequest request = + createRequestWithHeaders(HttpMethod.GET, TestingUri.SetFailureReasonInResponseWithNullMessage.toString()); + HttpUtil.setKeepAlive(request, false); + + EmbeddedChannel channel = createEmbeddedChannel(); + channel.writeInbound(request); + + HttpResponse response = channel.readOutbound(); + assertEquals(HttpResponseStatus.SERVICE_UNAVAILABLE, response.status()); + // No FAILURE_REASON_HEADER when root cause message is null — no NPE should be thrown + assertFalse("Should not have failure reason header when root message is null", + response.headers().contains(NettyResponseChannel.FAILURE_REASON_HEADER)); + assertFalse("Channel not closed on the server", channel.isActive()); + } + /** * Sends null input to {@link NettyResponseChannel#setHeader(String, Object)} (through * {@link MockNettyMessageProcessor}) and tests for reaction. @@ -1339,6 +1360,11 @@ enum TestingUri { * to the client. */ SetFailureReasonInResponseWithException, + /** + * When this request is received, a RestServiceException whose root cause has a null message is used on + * onResponseComplete. Verifies no NPE when the root cause message is null. + */ + SetFailureReasonInResponseWithNullMessage, /** * Catch all TestingUri. */ @@ -1584,6 +1610,12 @@ private void handleRequest(HttpRequest httpRequest) throws Exception { request.setArg(RestUtils.InternalKeys.SEND_FAILURE_REASON, Boolean.TRUE); restResponseChannel.onResponseComplete(new Exception(TestingUri.SetFailureReasonInResponse.toString())); break; + case SetFailureReasonInResponseWithNullMessage: + request.setArg(RestUtils.InternalKeys.SEND_FAILURE_REASON, Boolean.TRUE); + // TimeoutException() with no message simulates CompletableFuture.orTimeout() behavior + restResponseChannel.onResponseComplete(new RestServiceException("outer message", + new TimeoutException(), RestServiceErrorCode.ServiceUnavailable)); + break; } }