From abd1a436fda7ad762f59befc21ee378a4ba0054f Mon Sep 17 00:00:00 2001 From: jansupol Date: Wed, 8 Jun 2022 16:00:56 +0200 Subject: [PATCH] Call CompletionCallback just once with DefaultExceptionMapper Do not propagate error to the client Signed-off-by: jansupol --- .../jersey/server/DefaultExceptionMapper.java | 7 +- .../jersey/server/ServerRuntime.java | 22 +++--- .../server/DefaultExceptionMapperTest.java | 75 +++++++++++++++++++ 3 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 core-server/src/test/java/org/glassfish/jersey/server/DefaultExceptionMapperTest.java diff --git a/core-server/src/main/java/org/glassfish/jersey/server/DefaultExceptionMapper.java b/core-server/src/main/java/org/glassfish/jersey/server/DefaultExceptionMapper.java index b120e699a7..bd7c4d18f0 100644 --- a/core-server/src/main/java/org/glassfish/jersey/server/DefaultExceptionMapper.java +++ b/core-server/src/main/java/org/glassfish/jersey/server/DefaultExceptionMapper.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -19,6 +19,7 @@ import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.ext.ExceptionMapper; +import org.glassfish.jersey.server.internal.LocalizationMessages; class DefaultExceptionMapper implements ExceptionMapper { @Override @@ -35,6 +36,8 @@ private static Response processWebApplicationException(WebApplicationException e } private static Response processDefaultException(Throwable exception) { - return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(exception.getMessage()).build(); + return Response.status(Response.Status.INTERNAL_SERVER_ERROR) + .entity(LocalizationMessages.ERROR_EXCEPTION_MAPPING_THROWN_TO_CONTAINER()) + .build(); } } diff --git a/core-server/src/main/java/org/glassfish/jersey/server/ServerRuntime.java b/core-server/src/main/java/org/glassfish/jersey/server/ServerRuntime.java index 2e3a2635d1..007c9a5c49 100644 --- a/core-server/src/main/java/org/glassfish/jersey/server/ServerRuntime.java +++ b/core-server/src/main/java/org/glassfish/jersey/server/ServerRuntime.java @@ -365,11 +365,11 @@ public Responder(final RequestProcessingContext processingContext, final ServerR public void process(ContainerResponse response) { processingContext.monitoringEventBuilder().setContainerResponse(response); - response = processResponse(response); + response = processResponse(response, null); release(response); } - private ContainerResponse processResponse(ContainerResponse response) { + private ContainerResponse processResponse(ContainerResponse response, Throwable unmappedThrowable) { final Stage respondingRoot = processingContext.createRespondingRoot(); if (respondingRoot != null) { @@ -379,7 +379,7 @@ private ContainerResponse processResponse(ContainerResponse response) { // no-exception zone // the methods below are guaranteed to not throw any exceptions - completionCallbackRunner.onComplete(null); + completionCallbackRunner.onComplete(unmappedThrowable); return response; } @@ -427,7 +427,7 @@ public void process(final Throwable throwable) { final Response exceptionResponse = mapException(throwable); try { response = preProcessResponse(exceptionResponse, request); - processResponse(response); + processResponse(response, null); } catch (final Throwable respError) { LOGGER.log(Level.SEVERE, LocalizationMessages.ERROR_PROCESSING_RESPONSE_FROM_ALREADY_MAPPED_EXCEPTION()); processingContext.monitoringEventBuilder() @@ -448,9 +448,10 @@ public void process(final Throwable throwable) { try { request.getResponseWriter().failure(responseError); } finally { - completionCallbackRunner.onComplete(responseError); - defaultMapperResponse = processResponseWithDefaultExceptionMapper(responseError, request); + + // completionCallbackRunner.onComplete(responseError); is called from + // processResponseWithDefaultExceptionMapper } } @@ -488,7 +489,7 @@ private boolean processResponseError(final Throwable responseError) { if (processedError != null) { processedResponse = - processResponse(new ContainerResponse(processingContext.request(), processedError)); + processResponse(new ContainerResponse(processingContext.request(), processedError), null); processed = true; } } catch (final Throwable throwable) { @@ -545,10 +546,7 @@ private Response mapException(final Throwable originalThrowable) throws Throwabl final long timestamp = tracingLogger.timestamp(ServerTraceEvent.EXCEPTION_MAPPING); final ExceptionMapper mapper = runtime.exceptionMappers.findMapping(throwable); - if (mapper != null - && !DefaultExceptionMapper.class.getName() - .equals(mapper.getClass().getName()) - ) { + if (mapper != null && !DefaultExceptionMapper.class.isInstance(mapper)) { return processExceptionWithMapper(mapper, throwable, timestamp); } if (waeResponse != null) { @@ -625,7 +623,7 @@ private ContainerResponse processResponseWithDefaultExceptionMapper(Throwable ex ContainerRequest request) { long timestamp = tracingLogger.timestamp(ServerTraceEvent.EXCEPTION_MAPPING); final Response response = processExceptionWithMapper(DEFAULT_EXCEPTION_MAPPER, exception, timestamp); - return processResponse(preProcessResponse(response, request)); + return processResponse(preProcessResponse(response, request), exception); } private ContainerResponse writeResponse(final ContainerResponse response) { diff --git a/core-server/src/test/java/org/glassfish/jersey/server/DefaultExceptionMapperTest.java b/core-server/src/test/java/org/glassfish/jersey/server/DefaultExceptionMapperTest.java new file mode 100644 index 0000000000..74b11537f5 --- /dev/null +++ b/core-server/src/test/java/org/glassfish/jersey/server/DefaultExceptionMapperTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package org.glassfish.jersey.server; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.container.AsyncResponse; +import jakarta.ws.rs.container.CompletionCallback; +import jakarta.ws.rs.container.Suspended; +import jakarta.ws.rs.core.Response; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; + +public class DefaultExceptionMapperTest { + public static final String MESSAGE = "DefaultExceptionMapperTest I/O Exception"; + @Test + public void testIOException() { + IOException ioe = new IOException(MESSAGE); + DefaultExceptionMapper mapper = new DefaultExceptionMapper(); + Response response = mapper.toResponse(ioe); + Assert.assertFalse(response.getEntity().toString().contains(MESSAGE)); + } + + @Test + public void testCompletionCallback() { + AtomicInteger counter = new AtomicInteger(); + CompletionCallback hitOnceCallback = new CompletionCallback() { + @Override + public void onComplete(Throwable throwable) { + counter.incrementAndGet(); + } + }; + ResourceConfig resourceConfig = new ResourceConfig().register(new IOExThrowingResource(hitOnceCallback)); + ApplicationHandler applicationHandler = new ApplicationHandler(resourceConfig); + try { + applicationHandler.apply(RequestContextBuilder.from("/", "GET").build()).get(); + } catch (Exception e) { + // expected + } + + Assert.assertEquals(1, counter.get()); + } + + @Path("/") + public static class IOExThrowingResource { + private final CompletionCallback callback; + + public IOExThrowingResource(CompletionCallback callback) { + this.callback = callback; + } + + @GET + public String doGet(@Suspended AsyncResponse asyncResponse) throws IOException { + asyncResponse.register(callback); + throw new IOException(MESSAGE); + } + } +}