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

Netty Connector doesn't support Followredirects #5048

Merged
merged 2 commits into from
May 17, 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) 2016, 2020 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 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 @@ -18,12 +18,14 @@

import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeoutException;

import javax.ws.rs.core.Response;

import org.glassfish.jersey.client.ClientProperties;
import org.glassfish.jersey.client.ClientRequest;
import org.glassfish.jersey.client.ClientResponse;
import org.glassfish.jersey.netty.connector.internal.NettyInputStream;
Expand All @@ -35,6 +37,7 @@
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpObject;
import io.netty.handler.codec.http.HttpResponse;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http.HttpUtil;
import io.netty.handler.codec.http.LastHttpContent;
import io.netty.handler.timeout.IdleStateEvent;
Expand All @@ -46,21 +49,27 @@
*/
class JerseyClientHandler extends SimpleChannelInboundHandler<HttpObject> {

private static final String LOCATION_HEADER = "Location";
Copy link
Contributor

Choose a reason for hiding this comment

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

javax.ws.rs.core.HttpHeaders.Location


private final ClientRequest jerseyRequest;
private final CompletableFuture<ClientResponse> responseAvailable;
private final CompletableFuture<?> responseDone;
private final boolean followRedirects;
private final NettyConnector connector;

private NettyInputStream nis;
private ClientResponse jerseyResponse;

private boolean readTimedOut;

JerseyClientHandler(ClientRequest request,
CompletableFuture<ClientResponse> responseAvailable,
CompletableFuture<?> responseDone) {
JerseyClientHandler(ClientRequest request, CompletableFuture<ClientResponse> responseAvailable,
CompletableFuture<?> responseDone, NettyConnector connector) {
this.jerseyRequest = request;
this.responseAvailable = responseAvailable;
this.responseDone = responseDone;
// Follow redirects by default
this.followRedirects = jerseyRequest.resolveProperty(ClientProperties.FOLLOW_REDIRECTS, true);
this.connector = connector;
}

@Override
Expand All @@ -83,15 +92,36 @@ protected void notifyResponse() {
if (jerseyResponse != null) {
ClientResponse cr = jerseyResponse;
jerseyResponse = null;
responseAvailable.complete(cr);
int responseStatus = cr.getStatus();
if (followRedirects
&& (responseStatus == HttpResponseStatus.MOVED_PERMANENTLY.code()
|| responseStatus == HttpResponseStatus.FOUND.code()
|| responseStatus == HttpResponseStatus.SEE_OTHER.code()
|| responseStatus == HttpResponseStatus.TEMPORARY_REDIRECT.code()
|| responseStatus == HttpResponseStatus.PERMANENT_REDIRECT.code())) {
String location = cr.getHeaderString(LOCATION_HEADER);
try {
URI newUri = URI.create(location);
ClientRequest newReq = new ClientRequest(jerseyRequest);
newReq.setUri(newUri);
// Do not complete responseAvailable and try with new URI
// FIXME: This loops forever if HTTP response code is always a redirect.
// Currently there is no client property to specify a limit of redirections.
connector.execute(newReq, responseAvailable);
} catch (RuntimeException e) {
// It could happen if location header is wrong
responseAvailable.completeExceptionally(e);
}
} else {
responseAvailable.complete(cr);
}
}
}

@Override
public void channelRead0(ChannelHandlerContext ctx, HttpObject msg) {
if (msg instanceof HttpResponse) {
final HttpResponse response = (HttpResponse) msg;

jerseyResponse = new ClientResponse(new Response.StatusType() {
@Override
public int getStatusCode() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ class NettyConnector implements Connector {
@Override
public ClientResponse apply(ClientRequest jerseyRequest) {
try {
return execute(jerseyRequest).join();
CompletableFuture<ClientResponse> response = new CompletableFuture<>();
execute(jerseyRequest, response);
return response.join();
} catch (CompletionException cex) {
final Throwable t = cex.getCause() == null ? cex : cex.getCause();
throw new ProcessingException(t.getMessage(), t);
Expand All @@ -162,19 +164,25 @@ public ClientResponse apply(ClientRequest jerseyRequest) {

@Override
public Future<?> apply(final ClientRequest jerseyRequest, final AsyncConnectorCallback jerseyCallback) {
return execute(jerseyRequest).whenCompleteAsync((r, th) -> {
if (th == null) jerseyCallback.response(r);
else jerseyCallback.failure(th);
}, executorService);
CompletableFuture<ClientResponse> response = new CompletableFuture<>();
response.whenCompleteAsync((r, th) -> {
if (th == null) {
jerseyCallback.response(r);
} else {
jerseyCallback.failure(th);
}
}, executorService);
execute(jerseyRequest, response);
return response;
}

protected CompletableFuture<ClientResponse> execute(final ClientRequest jerseyRequest) {
protected void execute(final ClientRequest jerseyRequest,
final CompletableFuture<ClientResponse> responseAvailable) {
Integer timeout = jerseyRequest.resolveProperty(ClientProperties.READ_TIMEOUT, 0);
if (timeout == null || timeout < 0) {
throw new ProcessingException(LocalizationMessages.WRONG_READ_TIMEOUT(timeout));
}

final CompletableFuture<ClientResponse> responseAvailable = new CompletableFuture<>();
final CompletableFuture<?> responseDone = new CompletableFuture<>();

final URI requestUri = jerseyRequest.getUri();
Expand Down Expand Up @@ -290,7 +298,7 @@ protected void initChannel(SocketChannel ch) throws Exception {
// assert: it is ok to abort the entire response, if responseDone is completed exceptionally - in particular, nothing
// will leak
final Channel ch = chan;
JerseyClientHandler clientHandler = new JerseyClientHandler(jerseyRequest, responseAvailable, responseDone);
JerseyClientHandler clientHandler = new JerseyClientHandler(jerseyRequest, responseAvailable, responseDone, this);
// read timeout makes sense really as an inactivity timeout
ch.pipeline().addLast(READ_TIMEOUT_HANDLER,
new IdleStateHandler(0, 0, timeout, TimeUnit.MILLISECONDS));
Expand Down Expand Up @@ -411,8 +419,6 @@ public void run() {
} catch (InterruptedException e) {
responseDone.completeExceptionally(e);
}

return responseAvailable;
}

private String buildPathWithQueryParameters(URI requestUri) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* 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.netty.connector;

import static org.junit.Assert.assertEquals;

import java.net.URI;
import java.util.logging.Logger;

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

import org.glassfish.jersey.client.ClientConfig;
import org.glassfish.jersey.client.ClientProperties;
import org.glassfish.jersey.logging.LoggingFeature;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.test.JerseyTest;
import org.junit.Test;

public class FollowRedirectsTest extends JerseyTest {

private static final Logger LOGGER = Logger.getLogger(FollowRedirectsTest.class.getName());
private static final String REDIRECT_URL = "http://localhost:9998/test";

@Path("/test")
public static class RedirectResource {
@GET
public String get() {
return "GET";
}

@GET
@Path("redirect")
public Response redirect() {
return Response.seeOther(URI.create(REDIRECT_URL)).build();
}
}

@Override
protected Application configure() {
ResourceConfig config = new ResourceConfig(RedirectResource.class);
config.register(new LoggingFeature(LOGGER, LoggingFeature.Verbosity.PAYLOAD_ANY));
return config;
}

@Override
protected void configureClient(ClientConfig config) {
config.property(ClientProperties.FOLLOW_REDIRECTS, false);
config.connectorProvider(new NettyConnectorProvider());
}

@Test
public void testDoFollow() {
final URI u = target().getUri();
ClientConfig config = new ClientConfig().property(ClientProperties.FOLLOW_REDIRECTS, true);
config.connectorProvider(new NettyConnectorProvider());
Client c = ClientBuilder.newClient(config);
WebTarget t = c.target(u);
Response r = t.path("test/redirect")
.request().get();
assertEquals(200, r.getStatus());
assertEquals("GET", r.readEntity(String.class));
c.close();
}

@Test
public void testDoFollowPerRequestOverride() {
WebTarget t = target("test/redirect");
t.property(ClientProperties.FOLLOW_REDIRECTS, true);
Response r = t.request().get();
assertEquals(200, r.getStatus());
assertEquals("GET", r.readEntity(String.class));
}

@Test
public void testDontFollow() {
WebTarget t = target("test/redirect");
assertEquals(303, t.request().get().getStatus());
}

@Test
public void testDontFollowPerRequestOverride() {
final URI u = target().getUri();
ClientConfig config = new ClientConfig().property(ClientProperties.FOLLOW_REDIRECTS, true);
config.connectorProvider(new NettyConnectorProvider());
Client client = ClientBuilder.newClient(config);
WebTarget t = client.target(u);
t.property(ClientProperties.FOLLOW_REDIRECTS, false);
Response r = t.path("test/redirect").request().get();
assertEquals(303, r.getStatus());
client.close();
}
}