From f3edbf9594b3cf9e645077bbf539eb989b8848d8 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sun, 9 Sep 2012 16:23:45 +0200 Subject: [PATCH] jetty-9 - HTTP client: Introduced protocol handlers to handle redirect and authentication. --- .../org/eclipse/jetty/client/HttpClient.java | 27 +++- .../jetty/client/HttpConversation.java | 38 ++++- .../eclipse/jetty/client/HttpReceiver.java | 16 +- .../jetty/client/HttpResponseException.java | 13 +- .../eclipse/jetty/client/ProtocolHandler.java | 29 ++++ .../jetty/client/RedirectProtocolHandler.java | 147 ++++++++++++++++++ .../client/RedirectionProtocolListener.java | 62 -------- .../util/ByteBufferContentProvider.java | 43 ++++- ...nTest.java => HttpClientRedirectTest.java} | 82 +++++++++- .../jetty/client/HttpClientStreamTest.java | 4 + .../eclipse/jetty/client/HttpClientTest.java | 9 +- 11 files changed, 379 insertions(+), 91 deletions(-) create mode 100644 jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandler.java create mode 100644 jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java delete mode 100644 jetty-client/src/main/java/org/eclipse/jetty/client/RedirectionProtocolListener.java rename jetty-client/src/test/java/org/eclipse/jetty/client/{RedirectionTest.java => HttpClientRedirectTest.java} (53%) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 5b17177ae2d..818c85a2964 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -30,6 +30,7 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.Future; import javax.net.ssl.SSLEngine; @@ -95,6 +96,7 @@ public class HttpClient extends AggregateLifeCycle private final ConcurrentMap destinations = new ConcurrentHashMap<>(); private final ConcurrentMap conversations = new ConcurrentHashMap<>(); + private final List handlers = new CopyOnWriteArrayList<>(); private final CookieStore cookieStore = new HttpCookieStore(); private volatile Executor executor; private volatile ByteBufferPool byteBufferPool; @@ -108,6 +110,7 @@ public class HttpClient extends AggregateLifeCycle private volatile int maxQueueSizePerAddress = 1024; private volatile int requestBufferSize = 4096; private volatile int responseBufferSize = 4096; + private volatile int maxRedirects = 8; private volatile SocketAddress bindAddress; private volatile long idleTimeout; @@ -139,6 +142,8 @@ public class HttpClient extends AggregateLifeCycle selectorManager = newSelectorManager(); addBean(selectorManager); + handlers.add(new RedirectProtocolHandler(this)); + super.doStart(); LOG.info("Started {}", this); @@ -340,6 +345,16 @@ public class HttpClient extends AggregateLifeCycle this.responseBufferSize = responseBufferSize; } + public int getMaxRedirects() + { + return maxRedirects; + } + + public void setMaxRedirects(int maxRedirects) + { + this.maxRedirects = maxRedirects; + } + protected void newConnection(HttpDestination destination, Callback callback) { SocketChannel channel = null; @@ -398,16 +413,14 @@ public class HttpClient extends AggregateLifeCycle LOG.debug("{} removed", conversation); } - public Response.Listener lookup(int status) + // TODO: find a better method name + public Response.Listener lookup(Request request, Response response) { - // TODO - switch (status) + for (ProtocolHandler handler : handlers) { - case 302: - case 303: - return new RedirectionProtocolListener(this); + if (handler.accept(request, response)) + return handler.getResponseListener(); } - return null; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java index 752d03d3265..43c8a502e35 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java @@ -18,13 +18,19 @@ package org.eclipse.jetty.client; +import java.util.Collections; +import java.util.Enumeration; +import java.util.Map; import java.util.Queue; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.util.Attributes; -public class HttpConversation +public class HttpConversation implements Attributes { + private final Map attributes = new ConcurrentHashMap<>(); private final Queue exchanges = new ConcurrentLinkedQueue<>(); private final HttpClient client; private final long id; @@ -66,6 +72,36 @@ public class HttpConversation client.removeConversation(this); } + @Override + public Object getAttribute(String name) + { + return attributes.get(name); + } + + @Override + public void setAttribute(String name, Object attribute) + { + attributes.put(name, attribute); + } + + @Override + public void removeAttribute(String name) + { + attributes.remove(name); + } + + @Override + public Enumeration getAttributeNames() + { + return Collections.enumeration(attributes.keySet()); + } + + @Override + public void clearAttributes() + { + attributes.clear(); + } + @Override public String toString() { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 1e86b220b20..341d6d132a5 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -99,16 +99,17 @@ public class HttpReceiver implements HttpParser.ResponseHandler { HttpExchange exchange = connection.getExchange(); HttpConversation conversation = exchange.conversation(); - - // Probe the protocol listeners - HttpClient client = connection.getHttpClient(); HttpResponse response = exchange.response(); - Response.Listener listener = client.lookup(status); + + response.version(version).status(status).reason(reason); + + // Probe the protocol handlers + HttpClient client = connection.getHttpClient(); + Response.Listener listener = client.lookup(exchange.request(), response); if (listener == null) listener = conversation.first().listener(); conversation.listener(listener); - response.version(version).status(status).reason(reason); LOG.debug("Receiving {}", response); notifyBegin(listener, response); @@ -231,8 +232,9 @@ public class HttpReceiver implements HttpParser.ResponseHandler public void badMessage(int status, String reason) { HttpExchange exchange = connection.getExchange(); - exchange.response().status(status).reason(reason); - fail(new HttpResponseException()); + HttpResponse response = exchange.response(); + response.status(status).reason(reason); + fail(new HttpResponseException("HTTP protocol violation: bad response", response)); } private void notifyBegin(Response.Listener listener, Response response) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java index 4aceda1de29..be210a2235e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponseException.java @@ -18,9 +18,20 @@ package org.eclipse.jetty.client; +import org.eclipse.jetty.client.api.Response; + public class HttpResponseException extends RuntimeException { - public HttpResponseException() + private final Response response; + + public HttpResponseException(String message, Response response) { + super(message); + this.response = response; + } + + public Response getResponse() + { + return response; } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandler.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandler.java new file mode 100644 index 00000000000..64a0c90b14f --- /dev/null +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandler.java @@ -0,0 +1,29 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.client; + +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.client.api.Response; + +public interface ProtocolHandler +{ + public boolean accept(Request request, Response response); + + public Response.Listener getResponseListener(); +} diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java b/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java new file mode 100644 index 00000000000..265c7f44ac6 --- /dev/null +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectProtocolHandler.java @@ -0,0 +1,147 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.client; + +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.http.HttpFields; +import org.eclipse.jetty.http.HttpMethod; + +public class RedirectProtocolHandler extends Response.Listener.Adapter implements ProtocolHandler +{ + private static final String ATTRIBUTE = RedirectProtocolHandler.class.getName() + ".redirect"; + + private final HttpClient client; + + public RedirectProtocolHandler(HttpClient client) + { + this.client = client; + } + + @Override + public boolean accept(Request request, Response response) + { + switch (response.status()) + { + case 301: + case 302: + case 303: + case 307: + return true; + } + return false; + } + + @Override + public Response.Listener getResponseListener() + { + return this; + } + + @Override + public void onComplete(Result result) + { + if (!result.isFailed()) + { + Request request = result.getRequest(); + Response response = result.getResponse(); + String location = response.headers().get("location"); + int status = response.status(); + switch (status) + { + case 301: + { + if (request.method() == HttpMethod.GET || request.method() == HttpMethod.HEAD) + redirect(result, request.method(), location); + else + fail(result, new HttpResponseException("HTTP protocol violation: received 301 for non GET or HEAD request", response)); + break; + } + case 302: + case 303: + { + // Redirect must be done using GET + redirect(result, HttpMethod.GET, location); + break; + } + case 307: + { + // Keep same method + redirect(result, request.method(), location); + break; + } + default: + { + fail(result, new HttpResponseException("Unhandled HTTP status code " + status, response)); + break; + } + } + } + else + { + fail(result, result.getFailure()); + } + } + + private void redirect(Result result, HttpMethod method, String location) + { + Request request = result.getRequest(); + HttpConversation conversation = client.getConversation(request); + Integer redirects = (Integer)conversation.getAttribute(ATTRIBUTE); + if (redirects == null) + redirects = 0; + + if (redirects < client.getMaxRedirects()) + { + ++redirects; + conversation.setAttribute(ATTRIBUTE, redirects); + + Request redirect = client.newRequest(request.id(), location); + + // Use given method + redirect.method(method); + + // Copy headers + for (HttpFields.Field header : request.headers()) + redirect.header(header.getName(), header.getValue()); + + // Copy content + redirect.content(request.content()); + + redirect.send(new Adapter()); + } + else + { + fail(result, new HttpResponseException("Max redirects exceeded " + redirects, result.getResponse())); + } + } + + private void fail(Result result, Throwable failure) + { + Request request = result.getRequest(); + Response response = result.getResponse(); + HttpConversation conversation = client.getConversation(request); + Response.Listener listener = conversation.first().listener(); + // TODO: should we reply all event, or just the failure ? + // TODO: wrap these into try/catch as usual + listener.onFailure(response, failure); + listener.onComplete(new Result(request, response, failure)); + } +} diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectionProtocolListener.java b/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectionProtocolListener.java deleted file mode 100644 index e9f81955e48..00000000000 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/RedirectionProtocolListener.java +++ /dev/null @@ -1,62 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.client; - -import org.eclipse.jetty.client.api.Request; -import org.eclipse.jetty.client.api.Response; -import org.eclipse.jetty.client.api.Result; - -public class RedirectionProtocolListener extends Response.Listener.Adapter -{ - private final HttpClient client; - - public RedirectionProtocolListener(HttpClient client) - { - this.client = client; - } - - @Override - public void onComplete(Result result) - { - if (!result.isFailed()) - { - Response response = result.getResponse(); - switch (response.status()) - { - case 301: // GET or HEAD only allowed, keep the method - { - break; - } - case 302: - case 303: // use GET for next request - { - String location = response.headers().get("location"); - Request redirect = client.newRequest(result.getRequest().id(), location); - redirect.send(this); - break; - } - } - } - else - { - // TODO: here I should call on conversation.first().listener() both onFailure() and onComplete() - HttpConversation conversation = client.getConversation(result.getRequest()); - } - } -} diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java index cab165b99da..c5c6ae3f816 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/ByteBufferContentProvider.java @@ -19,32 +19,65 @@ package org.eclipse.jetty.client.util; import java.nio.ByteBuffer; -import java.util.Arrays; import java.util.Iterator; +import java.util.NoSuchElementException; import org.eclipse.jetty.client.api.ContentProvider; public class ByteBufferContentProvider implements ContentProvider { private final ByteBuffer[] buffers; + private final int length; public ByteBufferContentProvider(ByteBuffer... buffers) { this.buffers = buffers; + int length = 0; + for (ByteBuffer buffer : buffers) + length += buffer.remaining(); + this.length = length; } @Override public long length() { - int length = 0; - for (ByteBuffer buffer : buffers) - length += buffer.remaining(); return length; } @Override public Iterator iterator() { - return Arrays.asList(buffers).iterator(); + return new Iterator() + { + private int index; + + @Override + public boolean hasNext() + { + return index < buffers.length; + } + + @Override + public ByteBuffer next() + { + try + { + ByteBuffer buffer = buffers[index]; + buffers[index] = buffer.slice(); + ++index; + return buffer; + } + catch (ArrayIndexOutOfBoundsException x) + { + throw new NoSuchElementException(); + } + } + + @Override + public void remove() + { + throw new UnsupportedOperationException(); + } + }; } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/RedirectionTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientRedirectTest.java similarity index 53% rename from jetty-client/src/test/java/org/eclipse/jetty/client/RedirectionTest.java rename to jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientRedirectTest.java index 988b883a859..3f695e89d94 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/RedirectionTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientRedirectTest.java @@ -19,20 +19,28 @@ package org.eclipse.jetty.client; import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.client.util.ByteBufferContentProvider; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.toolchain.test.IO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -public class RedirectionTest extends AbstractHttpClientServerTest +import static org.junit.Assert.fail; + +public class HttpClientRedirectTest extends AbstractHttpClientServerTest { @Before public void init() throws Exception @@ -73,6 +81,76 @@ public class RedirectionTest extends AbstractHttpClientServerTest Assert.assertFalse(response.headers().containsKey(HttpHeader.LOCATION.asString())); } + @Test + public void test_301() throws Exception + { + Response response = client.newRequest("localhost", connector.getLocalPort()) + .method(HttpMethod.HEAD) + .path("/301/localhost/done") + .send().get(5, TimeUnit.SECONDS); + Assert.assertNotNull(response); + Assert.assertEquals(200, response.status()); + Assert.assertFalse(response.headers().containsKey(HttpHeader.LOCATION.asString())); + } + + @Test + public void test_301_WithWrongMethod() throws Exception + { + try + { + client.newRequest("localhost", connector.getLocalPort()) + .method(HttpMethod.POST) + .path("/301/localhost/done") + .send().get(5, TimeUnit.SECONDS); + fail(); + } + catch (ExecutionException x) + { + HttpResponseException xx = (HttpResponseException)x.getCause(); + Response response = xx.getResponse(); + Assert.assertNotNull(response); + Assert.assertEquals(301, response.status()); + Assert.assertTrue(response.headers().containsKey(HttpHeader.LOCATION.asString())); + } + } + + @Test + public void test_307_WithRequestContent() throws Exception + { + byte[] data = new byte[]{0, 1, 2, 3, 4, 5, 6, 7}; + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .method(HttpMethod.POST) + .path("/307/localhost/done") + .content(new ByteBufferContentProvider(ByteBuffer.wrap(data))) + .send().get(500, TimeUnit.SECONDS); + Assert.assertNotNull(response); + Assert.assertEquals(200, response.status()); + Assert.assertFalse(response.headers().containsKey(HttpHeader.LOCATION.asString())); + Assert.assertArrayEquals(data, response.content()); + } + + @Test + public void testMaxRedirections() throws Exception + { + client.setMaxRedirects(1); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .path("/303/localhost/302/localhost/done") + .send().get(5, TimeUnit.SECONDS); + fail(); + } + catch (ExecutionException x) + { + HttpResponseException xx = (HttpResponseException)x.getCause(); + Response response = xx.getResponse(); + Assert.assertNotNull(response); + Assert.assertEquals(302, response.status()); + Assert.assertTrue(response.headers().containsKey(HttpHeader.LOCATION.asString())); + } + } + private class RedirectHandler extends AbstractHandler { @Override @@ -89,6 +167,8 @@ public class RedirectionTest extends AbstractHttpClientServerTest catch (NumberFormatException x) { response.setStatus(200); + // Echo content back + IO.copy(request.getInputStream(), response.getOutputStream()); } finally { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientStreamTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientStreamTest.java index 051aadb1d79..8ea31a6c67a 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientStreamTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientStreamTest.java @@ -68,5 +68,9 @@ public class HttpClientStreamTest extends AbstractHttpClientServerTest Assert.assertEquals(200, response.status()); Assert.assertTrue(requestTime.get() <= responseTime); + + // Give some time to the server to consume the request content + // This is just to avoid exception traces in the test output + Thread.sleep(1000); } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 4f6cef93624..8eb976960cc 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.client; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.net.URLEncoder; import java.nio.ByteBuffer; @@ -47,6 +46,7 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.annotation.Slow; +import org.eclipse.jetty.util.IO; import org.junit.Assert; import org.junit.Test; @@ -393,12 +393,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { // Echo back - InputStream input = request.getInputStream(); - OutputStream output = response.getOutputStream(); - byte[] buffer = new byte[chunkSize]; - int read; - while ((read = input.read(buffer)) >= 0) - output.write(buffer, 0, read); + IO.copy(request.getInputStream(), response.getOutputStream()); } });