Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3442 from eclipse/jetty-9.4.x-3411-httpclient_timeout_infinite_redirect 

Fixes #3411 - HttpClient does not timeout during multiple redirection.
This commit is contained in:
Simone Bordet 2019-04-07 10:48:45 +02:00 committed by GitHub
parent f62192336f 23da561fec
commit 36700bec50
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 253 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
jetty-client/src/main/java/org/eclipse/jetty/client/AuthenticationProtocolHandler.java
View File

@ -23,6 +23,7 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@ -217,6 +218,8 @@ public abstract class AuthenticationProtocolHandler implements ProtocolHandler
path = request.getPath();
}
Request newRequest = client.copyRequest(request, requestURI);
// Disable the timeout so that only the one from the initial request applies.
newRequest.timeout(0, TimeUnit.MILLISECONDS);
if (path != null)
newRequest.path(path);

4
jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java
View File

@ -928,7 +928,7 @@ public class HttpClient extends ContainerLifeCycle
}
/**
* @return the max number of HTTP redirects that are followed
* @return the max number of HTTP redirects that are followed in a conversation
* @see #setMaxRedirects(int)
*/
public int getMaxRedirects()
@ -937,7 +937,7 @@ public class HttpClient extends ContainerLifeCycle
}
/**
* @param maxRedirects the max number of HTTP redirects that are followed
* @param maxRedirects the max number of HTTP redirects that are followed in a conversation, or -1 for unlimited redirects
* @see #setFollowRedirects(boolean)
*/
public void setMaxRedirects(int maxRedirects)

10
jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java
View File

@ -25,9 +25,13 @@ import java.util.concurrent.ConcurrentLinkedDeque;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.util.AttributesMap;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
public class HttpConversation extends AttributesMap
{
private static final Logger LOG = Log.getLogger(HttpConversation.class);
private final Deque<HttpExchange> exchanges = new ConcurrentLinkedDeque<>();
private volatile List<Response.ResponseListener> listeners;
@ -118,6 +122,7 @@ public class HttpConversation extends AttributesMap
HttpExchange lastExchange = exchanges.peekLast();
if (firstExchange == lastExchange)
{
// We don't have a conversation, just a single request.
if (overrideListener != null)
listeners.add(overrideListener);
else
@ -125,13 +130,16 @@ public class HttpConversation extends AttributesMap
}
else
{
// Order is important, we want to notify the last exchange first
// We have a conversation (e.g. redirect, authentication).
// Order is important, we want to notify the last exchange first.
listeners.addAll(lastExchange.getResponseListeners());
if (overrideListener != null)
listeners.add(overrideListener);
else
listeners.addAll(firstExchange.getResponseListeners());
}
if (LOG.isDebugEnabled())
LOG.debug("Exchanges in conversation {}, override={}, listeners={}", exchanges.size(), overrideListener, listeners);
this.listeners = listeners;
}

19
jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java
View File

@ -490,8 +490,12 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
exchanges.size(),
connectionPool);
}
// The TimeoutTask that expires when the next check of expiry is needed
/**
* This class enforces the total timeout for exchanges that are still in the queue.
* The total timeout for exchanges that are not in the destination queue is enforced
* by {@link HttpChannel}.
*/
private class TimeoutTask extends CyclicTimeout
{
private final AtomicLong nextTimeout = new AtomicLong(Long.MAX_VALUE);
@ -504,6 +508,9 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
@Override
public void onTimeoutExpired()
{
if (LOG.isDebugEnabled())
LOG.debug("{} timeout expired", this);
nextTimeout.set(Long.MAX_VALUE);
long now = System.nanoTime();
long nextExpiresAt = Long.MAX_VALUE;
@ -536,12 +543,16 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
if (timeoutAt != expiresAt)
{
long delay = expiresAt - System.nanoTime();
if (LOG.isDebugEnabled())
LOG.debug("Scheduled timeout in {} ms", TimeUnit.NANOSECONDS.toMillis(delay));
if (delay <= 0)
{
onTimeoutExpired();
}
else
{
schedule(delay, TimeUnit.NANOSECONDS);
if (LOG.isDebugEnabled())
LOG.debug("{} scheduled timeout in {} ms", this, TimeUnit.NANOSECONDS.toMillis(delay));
}
}
}
}

28
jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java
View File

@ -23,6 +23,7 @@ import java.net.URISyntaxException;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@ -61,10 +62,10 @@ public class HttpRedirector
{
private static final Logger LOG = Log.getLogger(HttpRedirector.class);
private static final String SCHEME_REGEXP = "(^https?)";
private static final String AUTHORITY_REGEXP = "([^/\\?#]+)";
private static final String AUTHORITY_REGEXP = "([^/?#]+)";
// The location may be relative so the scheme://authority part may be missing
private static final String DESTINATION_REGEXP = "(" + SCHEME_REGEXP + "://" + AUTHORITY_REGEXP + ")?";
private static final String PATH_REGEXP = "([^\\?#]*)";
private static final String PATH_REGEXP = "([^?#]*)";
private static final String QUERY_REGEXP = "([^#]*)";
private static final String FRAGMENT_REGEXP = "(.*)";
private static final Pattern URI_PATTERN = Pattern.compile(DESTINATION_REGEXP + PATH_REGEXP + QUERY_REGEXP + FRAGMENT_REGEXP);
@ -101,11 +102,11 @@ public class HttpRedirector
/**
* Redirects the given {@code response}, blocking until the redirect is complete.
*
* @param request the original request that triggered the redirect
* @param request the original request that triggered the redirect
* @param response the response to the original request
* @return a {@link Result} object containing the request to the redirected location and its response
* @throws InterruptedException if the thread is interrupted while waiting for the redirect to complete
* @throws ExecutionException if the redirect failed
* @throws ExecutionException if the redirect failed
* @see #redirect(Request, Response, Response.CompleteListener)
*/
public Result redirect(Request request, Response response) throws InterruptedException, ExecutionException
@ -144,7 +145,7 @@ public class HttpRedirector
/**
* Redirects the given {@code response} asynchronously.
*
* @param request the original request that triggered the redirect
* @param request the original request that triggered the redirect
* @param response the response to the original request
* @param listener the listener that receives response events
* @return the request to the redirected location
@ -292,7 +293,8 @@ public class HttpRedirector
Integer redirects = (Integer)conversation.getAttribute(ATTRIBUTE);
if (redirects == null)
redirects = 0;
if (redirects < client.getMaxRedirects())
int maxRedirects = client.getMaxRedirects();
if (maxRedirects < 0 || redirects < maxRedirects)
{
++redirects;
conversation.setAttribute(ATTRIBUTE, redirects);
@ -310,19 +312,17 @@ public class HttpRedirector
try
{
Request redirect = client.copyRequest(httpRequest, location);
// Disable the timeout so that only the one from the initial request applies.
redirect.timeout(0, TimeUnit.MILLISECONDS);
// Use given method
redirect.method(method);
redirect.onRequestBegin(new Request.BeginListener()
redirect.onRequestBegin(request ->
{
@Override
public void onBegin(Request redirect)
{
Throwable cause = httpRequest.getAbortCause();
if (cause != null)
redirect.abort(cause);
}
Throwable cause = httpRequest.getAbortCause();
if (cause != null)
request.abort(cause);
});
redirect.send(listener);

13
jetty-client/src/main/java/org/eclipse/jetty/client/TimeoutCompleteListener.java
View File

@ -26,7 +26,6 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.io.CyclicTimeout;
import org.eclipse.jetty.util.component.Destroyable;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.thread.Scheduler;
@ -47,7 +46,7 @@ public class TimeoutCompleteListener extends CyclicTimeout implements Response.C
{
Request request = this.request.getAndSet(null);
if (LOG.isDebugEnabled())
LOG.debug("Total timeout {} ms elapsed for {}", request.getTimeout(), request);
LOG.debug("Total timeout {} ms elapsed for {} on {}", request.getTimeout(), request, this);
if (request != null)
request.abort(new TimeoutException("Total timeout " + request.getTimeout() + " ms elapsed"));
}
@ -60,7 +59,7 @@ public class TimeoutCompleteListener extends CyclicTimeout implements Response.C
{
boolean cancelled = cancel();
if (LOG.isDebugEnabled())
LOG.debug("Cancelled ({}) timeout for {}", cancelled, request);
LOG.debug("Cancelled ({}) timeout for {} on {}", cancelled, request, this);
}
}
@ -69,12 +68,16 @@ public class TimeoutCompleteListener extends CyclicTimeout implements Response.C
if (this.request.compareAndSet(null, request))
{
long delay = timeoutAt - System.nanoTime();
if (LOG.isDebugEnabled())
LOG.debug("Scheduled timeout in {} ms for {}", TimeUnit.NANOSECONDS.toMillis(delay), request);
if (delay <= 0)
{
onTimeoutExpired();
}
else
{
schedule(delay, TimeUnit.NANOSECONDS);
if (LOG.isDebugEnabled())
LOG.debug("Scheduled timeout in {} ms for {} on {}", TimeUnit.NANOSECONDS.toMillis(delay), request, this);
}
}
}
}

6
jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java
View File

@ -261,12 +261,14 @@ public interface Request
Request idleTimeout(long timeout, TimeUnit unit);
/**
* @return the total timeout for this request, in milliseconds
* @return the total timeout for this request, in milliseconds;
* zero or negative if the timeout is disabled
*/
long getTimeout();
/**
* @param timeout the total timeout for the request/response conversation
* @param timeout the total timeout for the request/response conversation;
* use zero or a negative value to disable the timeout
* @param unit the timeout unit
* @return this request object
*/

173
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientAuthenticationTest.java
View File

@ -44,9 +44,11 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Response.Listener;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.util.AbstractAuthentication;
import org.eclipse.jetty.client.util.BasicAuthentication;
import org.eclipse.jetty.client.util.DeferredContentProvider;
import org.eclipse.jetty.client.util.DigestAuthentication;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.security.Authenticator;
import org.eclipse.jetty.security.ConstraintMapping;
@ -57,7 +59,6 @@ import org.eclipse.jetty.security.authentication.BasicAuthenticator;
import org.eclipse.jetty.security.authentication.DigestAuthenticator;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.IO;
@ -67,6 +68,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;
import static org.eclipse.jetty.client.api.Authentication.ANY_REALM;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalToIgnoringCase;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -137,7 +139,7 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
{
startBasic(scenario, new EmptyServerHandler());
URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort());
test_Authentication(scenario, new BasicAuthentication(uri, Authentication.ANY_REALM, "basic", "basic"));
test_Authentication(scenario, new BasicAuthentication(uri, ANY_REALM, "basic", "basic"));
}
@ParameterizedTest
@ -155,7 +157,7 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
{
startDigest(scenario, new EmptyServerHandler());
URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort());
test_Authentication(scenario, new DigestAuthentication(uri, Authentication.ANY_REALM, "digest", "digest"));
test_Authentication(scenario, new DigestAuthentication(uri, ANY_REALM, "digest", "digest"));
}
private void test_Authentication(final Scenario scenario, Authentication authentication) throws Exception
@ -227,16 +229,19 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
@ArgumentsSource(ScenarioProvider.class)
public void test_BasicAuthentication_ThenRedirect(Scenario scenario) throws Exception
{
startBasic(scenario, new AbstractHandler()
startBasic(scenario, new EmptyServerHandler()
{
private final AtomicInteger requests = new AtomicInteger();
@Override
public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
baseRequest.setHandled(true);
if (requests.incrementAndGet() == 1)
response.sendRedirect(URIUtil.newURI(scenario.getScheme(), request.getServerName(), request.getServerPort(), request.getRequestURI(), null));
int r = requests.incrementAndGet();
if (r == 1)
{
String path = request.getRequestURI() + "/" + r;
response.sendRedirect(URIUtil.newURI(scenario.getScheme(), request.getServerName(), request.getServerPort(), path, null));
}
}
});
@ -269,12 +274,11 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
@ArgumentsSource(ScenarioProvider.class)
public void test_Redirect_ThenBasicAuthentication(Scenario scenario) throws Exception
{
startBasic(scenario, new AbstractHandler()
startBasic(scenario, new EmptyServerHandler()
{
@Override
public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
baseRequest.setHandled(true);
if (request.getRequestURI().endsWith("/redirect"))
response.sendRedirect(URIUtil.newURI(scenario.getScheme(), request.getServerName(), request.getServerPort(), "/secure", null));
}
@ -571,61 +575,57 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
assertTrue(resultLatch.await(5, TimeUnit.SECONDS));
}
private static class GeneratingContentProvider implements ContentProvider
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void test_InfiniteAuthentication(Scenario scenario) throws Exception
{
private static final ByteBuffer DONE = ByteBuffer.allocate(0);
private final IntFunction<ByteBuffer> generator;
private GeneratingContentProvider(IntFunction<ByteBuffer> generator)
String authType = "Authenticate";
start(scenario, new EmptyServerHandler()
{
this.generator = generator;
}
@Override
public long getLength()
{
return -1;
}
@Override
public boolean isReproducible()
{
return true;
}
@Override
public Iterator<ByteBuffer> iterator()
{
return new Iterator<ByteBuffer>()
@Override
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
private int index;
public ByteBuffer current;
// Always reply with a 401 to see if the client
// can handle an infinite authentication loop.
response.setStatus(HttpStatus.UNAUTHORIZED_401);
response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), authType);
}
});
@Override
@SuppressWarnings("ReferenceEquality")
public boolean hasNext()
AuthenticationStore authenticationStore = client.getAuthenticationStore();
URI uri = URI.create(scenario.getScheme() + "://localhost:" + connector.getLocalPort());
authenticationStore.addAuthentication(new AbstractAuthentication(uri, Authentication.ANY_REALM)
{
@Override
public String getType()
{
return authType;
}
@Override
public Result authenticate(Request request, ContentResponse response, HeaderInfo headerInfo, Attributes context)
{
return new Result()
{
if (current == null)
@Override
public URI getURI()
{
current = generator.apply(index++);
if (current == null)
current = DONE;
return uri;
}
return current != DONE;
}
@Override
public ByteBuffer next()
{
ByteBuffer result = current;
current = null;
if (result == null)
throw new NoSuchElementException();
return result;
}
};
}
@Override
public void apply(Request request)
{
}
};
}
});
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.send();
assertEquals(HttpStatus.UNAUTHORIZED_401, response.getStatus());
}
@Test
@ -801,4 +801,61 @@ public class HttpClientAuthenticationTest extends AbstractHttpClientServerTest
assertEquals("thermostat", headerInfo.getParameter("realm"));
assertEquals(headerInfo.getParameter("nonce"), "1523430383=");
}
private static class GeneratingContentProvider implements ContentProvider
{
private static final ByteBuffer DONE = ByteBuffer.allocate(0);
private final IntFunction<ByteBuffer> generator;
private GeneratingContentProvider(IntFunction<ByteBuffer> generator)
{
this.generator = generator;
}
@Override
public long getLength()
{
return -1;
}
@Override
public boolean isReproducible()
{
return true;
}
@Override
public Iterator<ByteBuffer> iterator()
{
return new Iterator<ByteBuffer>()
{
private int index;
public ByteBuffer current;
@Override
@SuppressWarnings("ReferenceEquality")
public boolean hasNext()
{
if (current == null)
{
current = generator.apply(index++);
if (current == null)
current = DONE;
}
return current != DONE;
}
@Override
public ByteBuffer next()
{
ByteBuffer result = current;
current = null;
if (result == null)
throw new NoSuchElementException();
return result;
}
};
}
}
}

119
jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientRedirectTest.java
View File

@ -18,14 +18,6 @@
package org.eclipse.jetty.client;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.net.URLDecoder;
import java.nio.ByteBuffer;
@ -34,7 +26,9 @@ import java.nio.charset.StandardCharsets;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
@ -48,13 +42,20 @@ import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.toolchain.test.IO;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpClientRedirectTest extends AbstractHttpClientServerTest
{
@ParameterizedTest
@ -128,14 +129,13 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
{
start(scenario, new RedirectHandler());
ExecutionException x = assertThrows(ExecutionException.class, ()->{
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.method(HttpMethod.DELETE)
.path("/301/localhost/done")
.timeout(5, TimeUnit.SECONDS)
.send();
});
ExecutionException x = assertThrows(ExecutionException.class, () ->
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.method(HttpMethod.DELETE)
.path("/301/localhost/done")
.timeout(5, TimeUnit.SECONDS)
.send());
HttpResponseException xx = (HttpResponseException)x.getCause();
Response response = xx.getResponse();
assertNotNull(response);
@ -170,13 +170,12 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
start(scenario, new RedirectHandler());
client.setMaxRedirects(1);
ExecutionException x = assertThrows(ExecutionException.class, ()->{
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.path("/303/localhost/302/localhost/done")
.timeout(5, TimeUnit.SECONDS)
.send();
});
ExecutionException x = assertThrows(ExecutionException.class, () ->
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.path("/303/localhost/302/localhost/done")
.timeout(5, TimeUnit.SECONDS)
.send());
HttpResponseException xx = (HttpResponseException)x.getCause();
Response response = xx.getResponse();
assertNotNull(response);
@ -269,12 +268,11 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
@ArgumentsSource(ScenarioProvider.class)
public void testRedirectWithWrongScheme(Scenario scenario) throws Exception
{
start(scenario, new AbstractHandler()
start(scenario, new EmptyServerHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
baseRequest.setHandled(true);
response.setStatus(303);
response.setHeader("Location", "ssh://localhost:" + connector.getLocalPort() + "/path");
}
@ -439,12 +437,11 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
public void testRedirectWithCorruptedBody(Scenario scenario) throws Exception
{
byte[] bytes = "ok".getBytes(StandardCharsets.UTF_8);
start(scenario, new AbstractHandler()
start(scenario, new EmptyServerHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
baseRequest.setHandled(true);
if (target.startsWith("/redirect"))
{
response.setStatus(HttpStatus.SEE_OTHER_303);
@ -471,6 +468,60 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
assertArrayEquals(bytes, response.getContent());
}
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testRedirectToSameURL(Scenario scenario) throws Exception
{
start(scenario, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
response.setStatus(HttpStatus.SEE_OTHER_303);
response.setHeader(HttpHeader.LOCATION.asString(), request.getRequestURI());
}
});
ExecutionException x = assertThrows(ExecutionException.class, () ->
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.send());
assertThat(x.getCause(), Matchers.instanceOf(HttpResponseException.class));
}
@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testInfiniteRedirectLoopMustTimeout(Scenario scenario) throws Exception
{
AtomicLong counter = new AtomicLong();
start(scenario, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
try
{
Thread.sleep(200);
response.setStatus(HttpStatus.SEE_OTHER_303);
response.setHeader(HttpHeader.LOCATION.asString(), "/" + counter.getAndIncrement());
}
catch (InterruptedException x)
{
throw new RuntimeException(x);
}
}
});
assertThrows(TimeoutException.class, () ->
{
client.setMaxRedirects(-1);
client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.timeout(1, TimeUnit.SECONDS)
.send();
});
}
private void testSameMethodRedirect(final Scenario scenario, final HttpMethod method, int redirectCode) throws Exception
{
testMethodRedirect(scenario, method, method, redirectCode);
@ -519,10 +570,10 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
assertEquals(200, response.getStatus());
}
private class RedirectHandler extends AbstractHandler
private class RedirectHandler extends EmptyServerHandler
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
try
{
@ -551,10 +602,6 @@ public class HttpClientRedirectTest extends AbstractHttpClientServerTest
// Echo content back
IO.copy(request.getInputStream(), response.getOutputStream());
}
finally
{
baseRequest.setHandled(true);
}
}
}
}