You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by se...@apache.org on 2013/10/02 13:42:51 UTC

svn commit: r1528434 - in /cxf/trunk: core/src/main/java/org/apache/cxf/endpoint/ core/src/main/java/org/apache/cxf/transport/ rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/ rt/transports/http/src/main/java/org/apache/cxf/transport/http/ syste...

Author: sergeyb
Date: Wed Oct  2 11:42:51 2013
New Revision: 1528434

URL: http://svn.apache.org/r1528434
Log:
[CXF-5122] Removing a redundant MAX_AUTO_REDIRECT_COUNT property, initial update for getting the frontend clients in sync after the transport changes the target address

Modified:
    cxf/trunk/core/src/main/java/org/apache/cxf/endpoint/ClientImpl.java
    cxf/trunk/core/src/main/java/org/apache/cxf/transport/TransportURIResolver.java
    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/JAXRSClientServerBookTest.java

Modified: cxf/trunk/core/src/main/java/org/apache/cxf/endpoint/ClientImpl.java
URL: http://svn.apache.org/viewvc/cxf/trunk/core/src/main/java/org/apache/cxf/endpoint/ClientImpl.java?rev=1528434&r1=1528433&r2=1528434&view=diff
==============================================================================
--- cxf/trunk/core/src/main/java/org/apache/cxf/endpoint/ClientImpl.java (original)
+++ cxf/trunk/core/src/main/java/org/apache/cxf/endpoint/ClientImpl.java Wed Oct  2 11:42:51 2013
@@ -478,7 +478,7 @@ public class ClientImpl
                     public void onMessage(Message message) {
                         Exception ex = message.getContent(Exception.class);
                         if (ex != null) {
-                            getConduitSelector().complete(message.getExchange());
+                            completeExchange(message.getExchange());
                             if (message.getContent(Exception.class) == null) {
                                 // handle the right response
                                 List<Object> resList = null;
@@ -520,6 +520,15 @@ public class ClientImpl
         }
     }
 
+    private void completeExchange(Exchange exchange) {
+        getConduitSelector().complete(exchange);
+        Message outMessage = exchange.getOutMessage();
+        if (outMessage != null && outMessage.get("transport.retransmit.url") != null) {
+            //FIXME: target address has been updated at the transport level, 
+            //       update the the current client accordingly
+        }
+    }
+    
     /**
      * TODO This is SOAP specific code and should not be in cxf core
      * @param fault
@@ -560,7 +569,7 @@ public class ClientImpl
         }
         boolean mepCompleteCalled = false;
         if (ex != null) {
-            getConduitSelector().complete(exchange);
+            completeExchange(exchange);
             mepCompleteCalled = true;
             if (message.getContent(Exception.class) != null) {
                 throw ex;
@@ -569,7 +578,7 @@ public class ClientImpl
         ex = message.getExchange().get(Exception.class);
         if (ex != null) {
             if (!mepCompleteCalled) {
-                getConduitSelector().complete(exchange);
+                completeExchange(exchange);
             }
             throw ex;
         }
@@ -596,7 +605,7 @@ public class ClientImpl
         // leave the input stream open for the caller
         Boolean keepConduitAlive = (Boolean)exchange.get(Client.KEEP_CONDUIT_ALIVE);
         if (keepConduitAlive == null || !keepConduitAlive) {
-            getConduitSelector().complete(exchange);
+            completeExchange(exchange);
         }
 
         // Grab the response objects if there are any
@@ -730,7 +739,7 @@ public class ClientImpl
         try {
             if (callback != null) {
                 if (callback.isCancelled()) {
-                    getConduitSelector().complete(message.getExchange());
+                    completeExchange(message.getExchange());
                     return;
                 }
                 callback.start(message);

Modified: cxf/trunk/core/src/main/java/org/apache/cxf/transport/TransportURIResolver.java
URL: http://svn.apache.org/viewvc/cxf/trunk/core/src/main/java/org/apache/cxf/transport/TransportURIResolver.java?rev=1528434&r1=1528433&r2=1528434&view=diff
==============================================================================
--- cxf/trunk/core/src/main/java/org/apache/cxf/transport/TransportURIResolver.java (original)
+++ cxf/trunk/core/src/main/java/org/apache/cxf/transport/TransportURIResolver.java Wed Oct  2 11:42:51 2013
@@ -129,7 +129,7 @@ public class TransportURIResolver extend
                     InputStream ins = exch.get(InputStream.class);
                     resourceOpened.addElement(ins);
                     InputSource src = new InputSource(ins);
-                    String u = (String)message.get("http.retransmit.url");
+                    String u = (String)message.get("transport.retransmit.url");
                     if (u == null) {
                         u = base.toString();
                     }

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=1528434&r1=1528433&r2=1528434&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 Wed Oct  2 11:42:51 2013
@@ -497,6 +497,12 @@ public abstract class AbstractClient imp
             // In some cases, such as the "upfront" load-balancing, etc, the retries
             // won't be executed so it is necessary to reset the base address 
             calculateNewRequestURI(URI.create(s), getCurrentURI(), proxy);
+            return;
+        } 
+        s = (String)exchange.getOutMessage().get("transport.retransmit.url");
+        if (s != null && !state.getBaseURI().toString().equals(s)) {
+            calculateNewRequestURI(URI.create(s), getCurrentURI(), proxy);
+            return;
         }
         
     }
@@ -574,7 +580,15 @@ public abstract class AbstractClient imp
         UriBuilder builder = new UriBuilderImpl().uri(newBaseURI);
         String basePath = reqURIPath.startsWith(baseURIPath) ? baseURIPath : getBaseURI().getRawPath(); 
         builder.path(reqURIPath.equals(basePath) ? "" : reqURIPath.substring(basePath.length()));
-        URI newRequestURI = builder.replaceQuery(requestURI.getRawQuery()).build();
+        
+        String newQuery = newBaseURI.getRawQuery();
+        if (newQuery == null) {
+            builder.replaceQuery(requestURI.getRawQuery());
+        } else {
+            builder.replaceQuery(newQuery);
+        }
+        
+        URI newRequestURI = builder.build();
         
         resetBaseAddress(newBaseURI);
         URI current = proxy ? newBaseURI : newRequestURI; 

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=1528434&r1=1528433&r2=1528434&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 Wed Oct  2 11:42:51 2013
@@ -166,7 +166,7 @@ public abstract class HTTPConduit 
     
     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 MAX_AUTO_REDIRECT_COUNT = "max.http.redirect.count";
+    private static final String AUTO_REDIRECT_ALLOWED_URI = "http.redirect.allowed.uri";
     
     
     private static final String HTTP_POST_METHOD = "POST";
@@ -1193,7 +1193,7 @@ public abstract class HTTPConduit 
         
         protected void retransmit(String newURL) throws IOException {
             setupNewConnection(newURL);
-            outMessage.put("http.retransmit.url", newURL);
+            outMessage.put("transport.retransmit.url", newURL);
             if (cachedStream != null && cachedStream.size() < Integer.MAX_VALUE) {
                 setFixedLengthStreamingMode((int)cachedStream.size());
             }
@@ -1414,7 +1414,7 @@ public abstract class HTTPConduit 
             try {
                 newURL = convertToAbsoluteUrlIfNeeded(conduitName, urlString, newURL, outMessage);
                 detectRedirectLoop(conduitName, urlString, newURL, outMessage);
-                checkSameBaseUriRedirect(conduitName, urlString, newURL, outMessage);
+                checkAllowedRedirectUri(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
@@ -1727,24 +1727,36 @@ 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))) {
+    private static void checkAllowedRedirectUri(String conduitName,
+                                                String lastURL, 
+                                                String newURL,
+                                                Message message) throws IOException {
+        if (newURL != null) { 
             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 + "'";
+            if (MessageUtils.isTrue(message.getContextualProperty(AUTO_REDIRECT_SAME_HOST_ONLY))) {
+            
+                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);
+                }
+            }
+            
+            String allowedRedirectURI = (String)message.getContextualProperty(AUTO_REDIRECT_ALLOWED_URI);
+            if (allowedRedirectURI != null && !newURL.startsWith(allowedRedirectURI)) {
+                String msg = "Forbidden Redirect URI " + newURL + "detected on Conduit '" + conduitName;
                 LOG.log(Level.INFO, msg);
                 throw new IOException(msg);
             }
+            
         }
     }
     
@@ -1779,18 +1791,7 @@ public abstract class HTTPConduit 
         if (visitedURLs == null) {
             visitedURLs = new HashSet<String>();
             message.put(KEY_VISITED_URLS, visitedURLs);
-        } else {
-            Object maxCountProp = message.getContextualProperty(MAX_AUTO_REDIRECT_COUNT);
-            if (maxCountProp != null) {
-                Integer maxCount = maxCountProp instanceof Integer 
-                    ? (Integer)maxCountProp : Integer.valueOf((String)maxCountProp);
-                if (visitedURLs.size() == maxCount) {    
-                    String msg = "Too many redirects detected on Conduit '" + conduitName + "'";
-                    LOG.log(Level.INFO, msg);
-                    throw new IOException(msg);
-                }
-            }
-        }
+        } 
         visitedURLs.add(lastURL);
         if (newURL != null && visitedURLs.contains(newURL)) {
             // See if we are being redirected in a loop as best we can,

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=1528434&r1=1528433&r2=1528434&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 Wed Oct  2 11:42:51 2013
@@ -123,11 +123,15 @@ public class JAXRSClientServerBookTest e
     public void testGetBookRelativeUriAutoRedirect() throws Exception {
         String address = "http://localhost:" + PORT + "/bookstore/redirect/relative?loop=false";
         WebClient wc = WebClient.create(address);
+        assertEquals(address, wc.getCurrentURI().toString());
         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());
+        
+        String newAddress = "http://localhost:" + PORT + "/bookstore/redirect/relative?redirect=true";
+        assertEquals(newAddress, wc.getCurrentURI().toString());
     }
     
     @Test