Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blocked thread if WebApplicationException is reused. #4097 #5035

Merged
merged 2 commits into from
Apr 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 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
Expand Down Expand Up @@ -233,6 +233,7 @@ public void failure(final Throwable error) {
}
} finally {
requestTimeoutHandler.close();
responseContext.completeExceptionally(error);
rethrow(error);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 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
Expand All @@ -25,13 +25,15 @@
import java.util.concurrent.CompletionStage;
import java.util.concurrent.ExecutionException;

import javax.validation.ValidationException;
import javax.ws.rs.ProcessingException;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.client.ResponseProcessingException;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;

import javax.validation.ValidationException;

import org.glassfish.jersey.message.internal.OutboundJaxrsResponse;
import org.glassfish.jersey.message.internal.OutboundMessageContext;
import org.glassfish.jersey.message.internal.TracingLogger;
import org.glassfish.jersey.server.ContainerRequest;
import org.glassfish.jersey.server.SubjectSecurityContext;
Expand Down Expand Up @@ -77,6 +79,14 @@ public final Response dispatch(Object resource, ContainerRequest request) throws
Response response = null;
try {
response = doDispatch(resource, request);
if (response instanceof OutboundJaxrsResponse) {
OutboundJaxrsResponse responseImpl = (OutboundJaxrsResponse) response;
OutboundMessageContext context = responseImpl.getContext();
if (context.isCommitted()) {
// Response was already committed. This response is being reused more than 1 time.
throw new ResponseProcessingException(response, LocalizationMessages.ERROR_RESPONSE_ALREADY_COMMITED());
}
}
} finally {
TracingLogger.getInstance(request).log(ServerTraceEvent.DISPATCH_RESPONSE, response);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2012, 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
Expand Down Expand Up @@ -71,6 +71,7 @@ error.resources.cannot.merge=Resources do not have the same path and cannot be m
error.request.abort.in.response.phase=The request cannot be aborted as it is already in the response processing phase.
error.request.set.entity.stream.in.response.phase=The entity stream cannot be set in the request as it is already in the response processing phase.
error.request.set.security.context.in.response.phase=The security context cannot be set in the request as it is already in the response processing phase.
error.response.already.commited=Response was already committed. This response is being reused more than 1 time.
Copy link
Contributor

@jansupol jansupol Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response entity was already committed. The Response entity is being reused more than once.
Is it more explanatory to users? Maybe not that much correct, though.

error.scanning.class.not.found=A class file of the class name, {0}, is identified but the class could not be found.
error.sub.resource.locator.more.resources=Sub resource locator returned {0} in the resource model. Exactly one resource must be returned.
error.suspending.chunked.output.response=Attempt to suspend a client connection associated with a chunked output has failed in the underlying container.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* 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.tests.e2e.server;

import static org.junit.Assert.assertEquals;

import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.client.WebTarget;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.Response;

import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
import org.glassfish.jersey.test.jdkhttp.JdkHttpServerTestContainerFactory;
import org.glassfish.jersey.test.spi.TestContainerException;
import org.glassfish.jersey.test.spi.TestContainerFactory;
import org.junit.Test;

public class Issue4097Test extends JerseyTest {

@Override
protected Application configure() {
return new ResourceConfig(Issue4097Resource.class);
}

@Override
protected TestContainerFactory getTestContainerFactory() throws TestContainerException {
return new JdkHttpServerTestContainerFactory();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default TestContainerFactory seems to work as well.

}

@Test
public void reuseResponse() throws InterruptedException {
WebTarget webTarget = target("/issue4097/reuseResponse");
assertEquals(410, webTarget.request().get().getStatus());
assertEquals(500, webTarget.request().get().getStatus());
assertEquals(500, webTarget.request().get().getStatus());
}

@Path("/issue4097")
public static class Issue4097Resource {

private static final Response RESPONSE =
Response.status(410).entity("test").build();

@GET
@Path("/reuseResponse")
public Response reuseResponse() {
// returning the same response is obviously wrong
return RESPONSE;
}
}

}