jetty-9 - HTTP client: Introduced protocol handlers to handle redirect and authentication.

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<String, HttpDestination> destinations = new ConcurrentHashMap<>();
     private final ConcurrentMap<Long, HttpConversation> conversations = new ConcurrentHashMap<>();
+    private final List<ProtocolHandler> 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<Connection> 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
+        for (ProtocolHandler handler : handlers)
-        switch (status)
         {
-            case 302:
+            if (handler.accept(request,  response))
-            case 303:
+                return handler.getResponseListener();
-                return new RedirectionProtocolListener(this);
         }
-
         return null;
     }
 

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<String, Object> attributes = new ConcurrentHashMap<>();
     private final Queue<HttpExchange> 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<String> getAttributeNames()
+    {
+        return Collections.enumeration(attributes.keySet());
+    }
+
+    @Override
+    public void clearAttributes()
+    {
+        attributes.clear();
+    }
+
     @Override
     public String toString()
     {

jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java

@@ -99,16 +99,17 @@ public class HttpReceiver implements HttpParser.ResponseHandler<ByteBuffer>
     {
         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<ByteBuffer>
     public void badMessage(int status, String reason)
     {
         HttpExchange exchange = connection.getExchange();
-        exchange.response().status(status).reason(reason);
+        HttpResponse response = exchange.response();
-        fail(new HttpResponseException());
+        response.status(status).reason(reason);
+        fail(new HttpResponseException("HTTP protocol violation: bad response", response));
     }
 
     private void notifyBegin(Response.Listener listener, Response response)

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;
     }
 }

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();
+}

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));
+    }
+}

jetty-client/src/main/java/org/eclipse/jetty/client/RedirectionProtocolListener.java

@@ -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());
-        }
-    }
-}

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<ByteBuffer> iterator()
     {
-        return Arrays.asList(buffers).iterator();
+        return new Iterator<ByteBuffer>()
+        {
+            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();
+            }
+        };
     }
 }

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
             {

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);
     }
 }

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();
+                IO.copy(request.getInputStream(), response.getOutputStream());
-                OutputStream output = response.getOutputStream();
-                byte[] buffer = new byte[chunkSize];
-                int read;
-                while ((read = input.read(buffer)) >= 0)
-                    output.write(buffer, 0, read);
             }
         });