Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1504921 - in /cxf/trunk:
 rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/
 rt/transports/http/src/main/java/org/apache/cxf/transport/http/
 systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/
Date: Fri, 19 Jul 2013 15:37:39 -0000
To: commits@cxf.apache.org
From: sergeyb@apache.org
Message-Id: <20130719153740.1B79523888A6@eris.apache.org>

Author: sergeyb
Date: Fri Jul 19 15:37:39 2013
New Revision: 1504921

URL: http://svn.apache.org/r1504921
Log:
[CXF-5122] Optional support for restricting redirects to same host and for relative redirects

Modified:
    cxf/trunk/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
    cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java
    cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java
    cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java

Modified: cxf/trunk/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java?rev=1504921&r1=1504920&r2=1504921&view=diff
==============================================================================
--- cxf/trunk/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java (original)
+++ cxf/trunk/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java Fri Jul 19 15:37:39 2013
@@ -553,12 +553,16 @@ public abstract class AbstractClient imp
     }
     
     protected void checkClientException(Message outMessage, Exception ex) throws Exception {
+        Throwable actualEx = ex instanceof Fault ? ((Fault)ex).getCause() : ex;
+        
         Integer responseCode = getResponseCode(outMessage.getExchange());
-        if (responseCode == null) {
-            if (ex instanceof ProcessingException) {
+        if (responseCode == null 
+            || actualEx instanceof IOException 
+                && outMessage.getExchange().get("client.redirect.exception") != null) {
+            if (actualEx instanceof ProcessingException) {
                 throw ex;
-            } else if (ex != null) {
-                throw new ProcessingException(ex);
+            } else if (actualEx != null) {
+                throw new ProcessingException(actualEx);
             } else if (!outMessage.getExchange().isOneWay() || cfg.isResponseExpectedForOneway()) {
                 waitForResponseCode(outMessage.getExchange());
             }

Modified: cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java?rev=1504921&r1=1504920&r2=1504921&view=diff
==============================================================================
--- cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java (original)
+++ cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java Fri Jul 19 15:37:39 2013
@@ -163,8 +163,12 @@ public abstract class HTTPConduit 
      * Endpoint Qname to give the configuration name of this conduit.
      */
     private static final String SC_HTTP_CONDUIT_SUFFIX = ".http-conduit";
-
-    private static final String HTTP_POST_METHOD = "POST"; 
+    
+    private static final String AUTO_REDIRECT_SAME_HOST_ONLY = "http.redirect.same.host.only";
+    private static final String AUTO_REDIRECT_ALLOW_REL_URI = "http.redirect.relative.uri";
+    
+    
+    private static final String HTTP_POST_METHOD = "POST";
     private static final String HTTP_PUT_METHOD = "PUT";
     private static final Set<String> KNOWN_HTTP_VERBS_WITH_NO_CONTENT = 
         new HashSet<String>(Arrays.asList(new String[]{"GET", "DELETE", "HEAD", "OPTIONS", "TRACE"}));
@@ -1402,9 +1406,21 @@ public abstract class HTTPConduit 
             }
             Message m = new MessageImpl();
             updateResponseHeaders(m);
+            
             String newURL = extractLocation(Headers.getSetProtocolHeaders(m));
-
-            detectRedirectLoop(getConduitName(), url.toString(), newURL, outMessage);
+            String urlString = url.toString();
+            
+            try {
+                detectRedirectLoop(conduitName, urlString, newURL, outMessage);
+                newURL = convertToAbsoluteUrlIfNeeded(conduitName, urlString, newURL, outMessage);
+                checkSameBaseUriRedirect(conduitName, urlString, newURL, outMessage);
+            } catch (IOException ex) {
+                // Consider introducing ClientRedirectException instead - it will require
+                // those client runtimes which want to check for it have a direct link to it
+                outMessage.getExchange().put("client.redirect.exception", "true");
+                throw ex;
+            }
+            
             if (newURL != null) {
                 new Headers(outMessage).removeAuthorizationHeaders();
                 
@@ -1710,6 +1726,64 @@ public abstract class HTTPConduit 
         }
     }
 
+    private static void checkSameBaseUriRedirect(String conduitName,
+                                                 String lastURL, 
+                                                 String newURL,
+                                                 Message message) throws IOException {
+        if (newURL != null 
+            && MessageUtils.isTrue(message.getContextualProperty(AUTO_REDIRECT_SAME_HOST_ONLY))) {
+            URI newUri = URI.create(newURL);
+            URI lastUri = URI.create(lastURL);
+            // This can be further restricted to make sure newURL completely contains lastURL
+            // though making sure the same HTTP scheme and host are preserved should be enough
+            
+            if (!newUri.getScheme().equals(lastUri.getScheme())
+                || !newUri.getHost().equals(lastUri.getHost())) {
+                String msg = "Different HTTP Scheme or Host Redirect detected on Conduit '" 
+                    + conduitName + "' on '" + newURL + "'";
+                LOG.log(Level.INFO, msg);
+                throw new IOException(msg);
+            }
+        }
+    }
+    
+    // http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-23#section-7.1.2
+    // Relative Location values are also supported 
+    private static String convertToAbsoluteUrlIfNeeded(String conduitName,
+                                                       String lastURL, 
+                                                       String newURL,
+                                                       Message message) throws IOException {
+        if (newURL != null && !newURL.startsWith("http")) {
+            
+            if (MessageUtils.isTrue(message.getContextualProperty(AUTO_REDIRECT_ALLOW_REL_URI))) {
+                
+                int queryInd = lastURL.lastIndexOf('?');
+                String query = queryInd == -1 ? null : lastURL.substring(queryInd); 
+                String newAbsURL = queryInd == -1 ? lastURL : lastURL.substring(0, queryInd);
+                if (newAbsURL.endsWith("/")) {
+                    newAbsURL = newAbsURL.substring(0, newAbsURL.length() - 1);
+                }
+                newAbsURL = newAbsURL + newURL;
+                if (query != null) {
+                    if (newAbsURL.lastIndexOf("?") != -1) {
+                        newAbsURL += "&";
+                        query = query.substring(1);
+                    }
+                    newAbsURL += query;
+                }    
+                return newAbsURL;
+            } else {
+                String msg = "Relative Redirect detected on Conduit '" 
+                    + conduitName + "' on '" + newURL + "'";
+                LOG.log(Level.INFO, msg);
+                throw new IOException(msg);
+            }
+        } else {        
+            return newURL;
+        }
+            
+    }
+    
     private static void detectRedirectLoop(String conduitName, 
                                            String lastURL, 
                                            String newURL,
@@ -1721,14 +1795,20 @@ public abstract class HTTPConduit 
             message.put(KEY_VISITED_URLS, visitedURLs);
         }
         visitedURLs.add(lastURL);
-        if (newURL != null && visitedURLs.contains(newURL)) {
-            // See if we are being redirected in a loop as best we can,
-            // using string equality on URL.
-            // We are in a redirect loop; -- bail
-            String msg = "Redirect loop detected on Conduit '" 
-                + conduitName + "' on '" + newURL + "'";
-            LOG.log(Level.INFO, msg);
-            throw new IOException(msg);
+        if (newURL != null) {
+            if (visitedURLs.contains(newURL)) {
+                // See if we are being redirected in a loop as best we can,
+                // using string equality on URL.
+                // We are in a redirect loop; -- bail
+                String msg = "Redirect loop detected on Conduit '" 
+                    + conduitName + "' on '" + newURL + "'";
+                LOG.log(Level.INFO, msg);
+                throw new IOException(msg);
+            }
+            // Important to prevent looping on relative URIs
+            if (!newURL.startsWith("http")) {
+                visitedURLs.add(newURL);
+            }
         }
     }   
     private static void detectAuthorizationLoop(String conduitName, Message message, 

Modified: cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java
URL: http://svn.apache.org/viewvc/cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java?rev=1504921&r1=1504920&r2=1504921&view=diff
==============================================================================
--- cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java (original)
+++ cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java Fri Jul 19 15:37:39 2013
@@ -149,6 +149,35 @@ public class BookStore {
     }
     
     @GET
+    @Path("/redirect")
+    public Response getBookRedirect(@QueryParam("redirect") Boolean done,
+                                    @QueryParam("sameuri") Boolean sameuri) {
+        if (done == null) {
+            String uri = sameuri.equals(Boolean.TRUE) 
+                ? ui.getAbsolutePathBuilder().queryParam("redirect", "true").build().toString()
+                : "http://otherhost/redirect";
+            return Response.status(303).header("Location", uri).build();
+        } else {
+            return Response.ok(new Book("CXF", 123L), "application/xml").build();
+        }
+    }
+    
+    @GET
+    @Path("/redirect/relative")
+    public Response getBookRedirectRel(@QueryParam("redirect") Boolean done,
+                                       @QueryParam("loop") boolean loop) {
+        if (done == null) {
+            if (loop) {
+                return Response.status(303).header("Location", "/?a").build();                
+            } else {
+                return Response.status(303).header("Location", "/?redirect=true").build();    
+            }
+        } else {
+            return Response.ok(new Book("CXF", 124L), "application/xml").build();
+        }
+    }
+    
+    @GET
     @Path("/booklist")
     public List<String> getBookListArray() {
         return Collections.singletonList("Good book");
@@ -1433,7 +1462,7 @@ public class BookStore {
         public void write(OutputStream output) throws IOException, WebApplicationException {
             if (failEarly) {
                 throw new WebApplicationException(
-                     Response.status(410).type("text/plain")
+                     Response.status(410).header("content-type", "text/plain")
                      .entity("This is supposed to go on the wire").build());
             } else {
                 output.write("This is not supposed to go on the wire".getBytes());

Modified: cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java
URL: http://svn.apache.org/viewvc/cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java?rev=1504921&r1=1504920&r2=1504921&view=diff
==============================================================================
--- cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java (original)
+++ cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java Fri Jul 19 15:37:39 2013
@@ -94,6 +94,72 @@ public class JAXRSClientServerBookTest e
     }
     
     @Test
+    public void testGetBookSameUriAutoRedirect() throws Exception {
+        String address = "http://localhost:" + PORT + "/bookstore/redirect?sameuri=true";
+        WebClient wc = WebClient.create(address);
+        WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+        Response r = wc.get();
+        Book book = r.readEntity(Book.class);
+        assertEquals(123L, book.getId());
+    }
+    
+    @Test
+    public void testGetBookDiffUriAutoRedirect() throws Exception {
+        String address = "http://localhost:" + PORT + "/bookstore/redirect?sameuri=false";
+        WebClient wc = WebClient.create(address);
+        WebClient.getConfig(wc).getRequestContext().put("http.redirect.same.host.only", "true");
+        WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+        try {
+            wc.get();
+            fail("Redirect to different host is not allowed");
+        } catch (ProcessingException ex) {
+            Throwable cause = ex.getCause();
+            assertTrue(cause.getMessage().startsWith("Different HTTP Scheme or Host Redirect detected on"));
+        }
+    }
+    
+
+    @Test
+    public void testGetBookRelativeUriAutoRedirect() throws Exception {
+        String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=false";
+        WebClient wc = WebClient.create(address);
+        WebClient.getConfig(wc).getRequestContext().put("http.redirect.relative.uri", "true");
+        WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+        Response r = wc.get();
+        Book book = r.readEntity(Book.class);
+        assertEquals(124L, book.getId());
+    }
+    
+    @Test
+    public void testGetBookRelativeUriAutoRedirectLoop() throws Exception {
+        String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=true";
+        WebClient wc = WebClient.create(address);
+        WebClient.getConfig(wc).getRequestContext().put("http.redirect.relative.uri", "true");
+        WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+        try {
+            wc.get();
+            fail("Redirect loop must be detected");
+        } catch (ProcessingException ex) {
+            Throwable cause = ex.getCause();
+            assertTrue(cause.getMessage().contains("Redirect loop detected on"));
+        }
+    }
+    
+    @Test
+    public void testGetBookRelativeUriAutoRedirectNotAllowed() throws Exception {
+        String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=true";
+        WebClient wc = WebClient.create(address);
+        WebClient.getConfig(wc).getHttpConduit().getClient().setAutoRedirect(true);
+        try {
+            wc.get();
+            fail("relative Redirect is not allowed");
+        } catch (ProcessingException ex) {
+            Throwable cause = ex.getCause();
+            assertTrue(cause.getMessage().startsWith("Relative Redirect detected on"));
+        }
+    }
+    
+    @Test
     public void testPostEmptyForm() throws Exception {
         String address = "http://localhost:" + PORT + "/bookstore/emptyform";
         WebClient wc = WebClient.create(address);