296567 HttpClient RedirectListener handles new HttpDestination

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@1872 7e9141cc-0065-0410-87d8-b60c137991c4

VERSION.txt

@@ -1,4 +1,5 @@
 jetty-7.1.3-SNAPSHOT
+ + 296567 HttpClient RedirectListener handles new HttpDestination
  + 297598 JDBCLoginService uses hardcoded credential class
  + 305898 Websocket handles query string in URI
  + 307457 Exchanges are left unhandled when connection is lost

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

@@ -296,7 +296,7 @@ public class HttpClient extends HttpBuffers implements Attributes
 
     /**
      * Registers a listener that can listen to the stream of execution between the client and the
-     * server and influence events.  Sequential calls to the method wrapper sequentially wrap the preceeding
+     * server and influence events.  Sequential calls to the method wrapper sequentially wrap the preceding
      * listener in a delegation model.
      * <p/>
      * NOTE: the SecurityListener is a special listener which doesn't need to be added via this
@@ -313,7 +313,8 @@ public class HttpClient extends HttpBuffers implements Attributes
         }
         _registeredListeners.add(listenerClass);
     }
-
+    
+    /* ------------------------------------------------------------ */
     public LinkedList<String> getRegisteredListeners()
     {
         return _registeredListeners;

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

@@ -426,7 +426,8 @@ public class HttpDestination
         }
     }
 
-    private void recycleConnection(HttpConnection connection) {
+    private void recycleConnection(HttpConnection connection) 
+    {
     	connection.cancelIdleTimeout();
     	try
         {

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

@@ -16,6 +16,7 @@ package org.eclipse.jetty.client;
 import java.io.IOException;
 
 import org.eclipse.jetty.http.HttpHeaders;
+import org.eclipse.jetty.http.HttpSchemes;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.io.Buffer;
 
@@ -26,8 +27,8 @@ import org.eclipse.jetty.io.Buffer;
  */
 public class RedirectListener extends HttpEventListenerWrapper
 {
+    private final HttpExchange _exchange;
     private HttpDestination _destination;
-    private HttpExchange _exchange;
     private String _location;
     private int _attempts;
     private boolean _requestComplete;
@@ -44,6 +45,7 @@ public class RedirectListener extends HttpEventListenerWrapper
         _exchange = ex;
     }
 
+    @Override
     public void onResponseStatus( Buffer version, int status, Buffer reason )
         throws IOException
     {
@@ -61,6 +63,7 @@ public class RedirectListener extends HttpEventListenerWrapper
     }
 
 
+    @Override
     public void onResponseHeader( Buffer name, Buffer value )
         throws IOException
     {
@@ -77,6 +80,7 @@ public class RedirectListener extends HttpEventListenerWrapper
         super.onResponseHeader(name,value);
     }
 
+    @Override
     public void onRequestComplete() throws IOException
     {
         _requestComplete = true;
@@ -87,6 +91,7 @@ public class RedirectListener extends HttpEventListenerWrapper
         }
     }
 
+    @Override
     public void onResponseComplete() throws IOException
     {
         _responseComplete = true;
@@ -109,7 +114,23 @@ public class RedirectListener extends HttpEventListenerWrapper
                 else
                     _exchange.setURI(_location);
 
-                _destination.resend(_exchange);
+                // destination may have changed
+                HttpDestination destination=_destination.getHttpClient().getDestination(_exchange.getAddress(),HttpSchemes.HTTPS.equals(String.valueOf(_exchange.getScheme())));
+                
+                if (_destination==destination)
+                    _destination.resend(_exchange);
+                else
+                {
+                    // unwrap to find ultimate listener.
+                    HttpEventListener listener=this;
+                    while(listener instanceof HttpEventListenerWrapper)
+                        listener=((HttpEventListenerWrapper)listener).getEventListener();
+                    //reset the listener
+                    _exchange.getEventListener().onRetry();
+                    _exchange.reset();
+                    _exchange.setEventListener(listener);
+                    destination.send(_exchange);
+                }
 
                 return false;
             }

jetty-client/src/test/java/org/eclipse/jetty/client/HttpGetRedirectTest.java

@@ -35,6 +35,7 @@ import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.server.nio.SelectChannelConnector;
+import org.eclipse.jetty.util.log.Log;
 
 
 /* ------------------------------------------------------------ */
@@ -63,6 +64,8 @@ public class HttpGetRedirectTest
     private Realm _realm;
     private String _protocol;
     private String _requestUrl;
+    private String _requestUrl2;
+    private RedirectHandler _handler;
 
     public void setUp()
         throws Exception
@@ -73,10 +76,14 @@ public class HttpGetRedirectTest
 
         _server = new Server();
         configureServer(_server);
+        org.eclipse.jetty.server.bio.SocketConnector connector = new org.eclipse.jetty.server.bio.SocketConnector();
+        _server.addConnector(connector);
         _server.start();
 
         int port = _server.getConnectors()[0].getLocalPort();
         _requestUrl = _protocol+"://localhost:"+port+ "/content.txt";
+        
+        _handler._toURL=_protocol+"://localhost:"+connector.getLocalPort()+ "/moved.txt";
     }
 
     public void tearDown()
@@ -92,7 +99,7 @@ public class HttpGetRedirectTest
     public void testGet() throws Exception
     {
         startClient(_realm);
-
+        
         ContentExchange getExchange = new ContentExchange();
         getExchange.setURL(_requestUrl);
         getExchange.setMethod(HttpMethods.GET);
@@ -107,10 +114,10 @@ public class HttpGetRedirectTest
             content = getExchange.getResponseContent();
         }
 
-        stopClient();
-
         assertEquals(HttpStatus.OK_200,responseStatus);
         assertEquals(_content,content);
+        
+        stopClient();
     }
 
     protected void configureServer(Server server)
@@ -121,8 +128,8 @@ public class HttpGetRedirectTest
         SelectChannelConnector connector = new SelectChannelConnector();
         server.addConnector(connector);
 
-        Handler handler = new RedirectHandler(HttpStatus.MOVED_PERMANENTLY_301, "/content.txt", "/moved.txt", 1);
+        _handler = new RedirectHandler(HttpStatus.MOVED_PERMANENTLY_301, "/content.txt", "WAIT FOR IT", 2);
-        server.setHandler( handler );
+        server.setHandler( _handler );
 
     }
 
@@ -162,46 +169,26 @@ public class HttpGetRedirectTest
         _realm = realm;
     }
 
-    public static void copyStream(InputStream in, OutputStream out)
-    {
-        try
-        {
-            byte[] buffer=new byte[1024];
-            int len;
-            while ((len=in.read(buffer))>=0)
-            {
-                out.write(buffer,0,len);
-            }
-        }
-        catch (EofException e)
-        {
-            System.err.println(e);
-        }
-        catch (IOException e)
-        {
-            e.printStackTrace();
-        }
-    }
 
     private static class RedirectHandler
         extends AbstractHandler
     {
-        private final String origUrl;
+        private final String _fromURI;
+        private final int _code;
+        private final int _maxRedirects;
+        private int _redirectCount = 0;
+        private String _toURL;
 
-        private final int code;
+        public RedirectHandler( final int code, final String fromURI, final String toURL, final int maxRedirects )
-
-        private final int maxRedirects;
-
-        private int redirectCount = 0;
-
-        private final String currUrl;
-
-        public RedirectHandler( final int code, final String currUrl, final String origUrl, final int maxRedirects )
         {
-            this.code = code;
+            this._code = code;
-            this.currUrl = currUrl;
+            this._fromURI = fromURI;
-            this.origUrl = origUrl;
+            this._toURL = toURL;
-            this.maxRedirects = maxRedirects;
+            this._maxRedirects = maxRedirects;
+            
+            if (_fromURI==null || _toURL==null)
+                throw new IllegalArgumentException();
+            
         }
 
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
@@ -212,23 +199,18 @@ public class HttpGetRedirectTest
                 return;
             }
 
-            if (request.getRequestURI().equals(currUrl))
+            if (request.getRequestURI().equals(_fromURI))
             {
-                redirectCount++;
+                _redirectCount++;
 
-                if ( maxRedirects < 0 || redirectCount <= maxRedirects )
+                String location = ( _redirectCount <= _maxRedirects )?_fromURI:_toURL;
-                {
+
-                    response.setStatus( code );
+                response.setStatus( _code );
-                    response.setHeader( "Location", currUrl );
+                response.setHeader( "Location", location );
-                }
+                
-                else
-                {
-                    response.setStatus( code );
-                    response.setHeader( "Location", origUrl );
-                }
                 ( (Request) request ).setHandled( true );
             }
-            else if (request.getRequestURI().equals(origUrl))
+            else
             {
                 PrintWriter out = response.getWriter();
                 out.write(_content);