Fix Issue #63. Fix the problem that NFHttpClient is registered with Servo monitors twice.

ribbon-core/src/main/java/com/netflix/client/LoadBalancerContext.java

@@ -19,12 +19,14 @@ package com.netflix.client;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URLEncoder;
 import java.util.Collection;
 import java.util.concurrent.TimeUnit;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Strings;
 import com.netflix.client.config.CommonClientConfigKey;
 import com.netflix.client.config.DefaultClientConfigImpl;
 import com.netflix.client.config.IClientConfig;
@@ -518,12 +520,35 @@ public abstract class LoadBalancerContext<T extends ClientRequest, S extends IRe
             }
             
             newURI = new URI(scheme, theUrl.getUserInfo(), host, port, urlPath, theUrl.getQuery(), theUrl.getFragment());
+            if (isURIEncoded(theUrl)) {
+                StringBuilder sb = new StringBuilder();
+                sb.append(newURI.getScheme())
+                  .append("://")
+                  .append(newURI.getRawAuthority())
+                  .append(theUrl.getRawPath());
+                if (!Strings.isNullOrEmpty(theUrl.getRawQuery())) {
+                    sb.append("?").append(theUrl.getRawQuery());
+                }
+                if (!Strings.isNullOrEmpty(theUrl.getRawFragment())) {
+                    sb.append("#").append(theUrl.getRawFragment());
+                }
+                newURI = new URI(sb.toString());
+            }
             return (T) original.replaceUri(newURI);            
         } catch (URISyntaxException e) {
             throw new ClientException(ClientException.ErrorType.GENERAL, e.getMessage());
         }
     }
 
+    private boolean isURIEncoded(URI uri) {
+        String original = uri.toString();
+        try {
+            return !URLEncoder.encode(original, "UTF-8").equals(original);
+        } catch (Exception e) {
+            return false;
+        }
+    }
+    
     protected boolean isRetriable(T request) {
         if (request.isRetriable()) {
             return true;            

ribbon-core/src/test/java/com/netflix/client/LoadBalancerContextTest.java

+/*
+ *
+ * Copyright 2013 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package com.netflix.client;
+
+import static org.junit.Assert.*;
+
+import java.net.URLEncoder;
+
+import org.junit.Test;
+
+import com.netflix.client.http.HttpRequest;
+import com.netflix.client.http.HttpResponse;
+import com.netflix.loadbalancer.BaseLoadBalancer;
+import com.netflix.loadbalancer.Server;
+
+public class LoadBalancerContextTest {
+
+    static BaseLoadBalancer lb = new BaseLoadBalancer() {
+
+        @Override
+        public Server chooseServer(Object key) {
+            return new Server("www.example.com:8080");
+        }
+    };
+    
+    
+    private MyLoadBalancerContext context;
+    
+    public LoadBalancerContextTest() {
+        context = new MyLoadBalancerContext();
+        context.setLoadBalancer(lb);
+    }
+    
+    @Test
+    public void testComputeFinalUriWithLoadBalancer() throws ClientException {
+        HttpRequest request = HttpRequest.newBuilder().uri("/test?abc=xyz").build();
+        HttpRequest newRequest = context.computeFinalUriWithLoadBalancer(request);
+        assertEquals("http://www.example.com:8080/test?abc=xyz", newRequest.getUri().toString());
+    }
+    
+    @Test
+    public void testEncodedPath() throws ClientException {
+        String uri = "http://localhost:8080/resources/abc%2Fxyz";
+        HttpRequest request = HttpRequest.newBuilder().uri(uri).build();
+        HttpRequest newRequest = context.computeFinalUriWithLoadBalancer(request);
+        assertEquals(uri, newRequest.getUri().toString());
+    }
+    
+    @Test
+    public void testEncodedPathAndHostChange() throws ClientException {
+        String uri = "/abc%2Fxyz";
+        HttpRequest request = HttpRequest.newBuilder().uri(uri).build();
+        HttpRequest newRequest = context.computeFinalUriWithLoadBalancer(request);
+        assertEquals("http://www.example.com:8080" + uri, newRequest.getUri().toString());
+    }
+
+    
+    @Test
+    public void testEncodedQuery() throws Exception {
+        String uri = "http://localhost:8080/resources/abc?";
+        String queryString = "name=" + URLEncoder.encode("éƎ&=*%!@#$%^&*()", "UTF-8");   
+        HttpRequest request = HttpRequest.newBuilder().uri(uri + queryString).build();
+        HttpRequest newRequest = context.computeFinalUriWithLoadBalancer(request);
+        assertEquals(uri + queryString, newRequest.getUri().toString());        
+    }
+}
+
+class MyLoadBalancerContext extends LoadBalancerContext<HttpRequest, HttpResponse> {
+}

ribbon-httpclient/src/main/java/com/netflix/http4/NFHttpClient.java

@@ -89,29 +89,30 @@ public class NFHttpClient extends DefaultHttpClient {
 		this.name = "UNNAMED_" + numNonNamedHttpClients.incrementAndGet();
 		httpHost = new HttpHost(host, port);
 		httpRoute = new HttpRoute(httpHost);
-		init(DefaultClientConfigImpl.getClientConfigWithDefaultValues());
+		init(DefaultClientConfigImpl.getClientConfigWithDefaultValues(), false);
 	}   
 
 	protected NFHttpClient(){
 		super(new ThreadSafeClientConnManager());
 		this.name = "UNNAMED_" + numNonNamedHttpClients.incrementAndGet();
-		init(DefaultClientConfigImpl.getClientConfigWithDefaultValues());
+		init(DefaultClientConfigImpl.getClientConfigWithDefaultValues(), false);
 	}
 
 	protected NFHttpClient(String name) {
-		super(new MonitoredConnectionManager(name));
-		this.name = name;
-		init(DefaultClientConfigImpl.getClientConfigWithDefaultValues());
+	    this(name, DefaultClientConfigImpl.getClientConfigWithDefaultValues(), true);
 	}
 
     protected NFHttpClient(String name, IClientConfig config) {
+        this(name, config, true);
+    }
+
+    protected NFHttpClient(String name, IClientConfig config, boolean registerMonitor) {
         super(new MonitoredConnectionManager(name));
         this.name = name;
-        init(config);
+        init(config, registerMonitor);
     }
-
 	
-	void init(IClientConfig config) {
+	void init(IClientConfig config, boolean registerMonitor) {
 		HttpParams params = getParams();
 
 		HttpProtocolParams.setContentCharset(params, "UTF-8");  
@@ -132,7 +133,9 @@ public class NFHttpClient extends DefaultHttpClient {
 				new NFHttpMethodRetryHandler(this.name, this.retriesProperty.get(), false,
 						this.sleepTimeFactorMsProperty.get()));
 	    tracer = Monitors.newTimer(EXECUTE_TRACER, TimeUnit.MILLISECONDS);
-        Monitors.registerObject(name, this);
+	    if (registerMonitor) {
+            Monitors.registerObject(name, this);
+	    }
 	}
 
 	public void initConnectionCleanerTask(){

ribbon-httpclient/src/main/java/com/netflix/http4/NFHttpClientFactory.java

 /*
-*
-* Copyright 2013 Netflix, Inc.
-*
-* Licensed under the Apache License, Version 2.0 (the "License");
-* you may not use this file except in compliance with the License.
-* You may obtain a copy of the License at
-*
-* http://www.apache.org/licenses/LICENSE-2.0
-*
-* Unless required by applicable law or agreed to in writing, software
-* distributed under the License is distributed on an "AS IS" BASIS,
-* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-* See the License for the specific language governing permissions and
-* limitations under the License.
-*
-*/
+ *
+ * Copyright 2013 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
 package com.netflix.http4;
 
 import java.util.Map;
@@ -71,11 +71,8 @@ public class NFHttpClientFactory {
 		    synchronized (lock) {
 		        client = namedClientMap.get(name);       
 		        if (client == null){
-        			client = new NFHttpClient(name, config);
+        			client = new NFHttpClient(name, config, registerMonitor);
         			namedClientMap.put(name,client);
-        			if (registerMonitor) {
-        			    Monitors.registerObject("HttpClient_" + name, client);
-        			}
 		        }
 		    }
 		}
@@ -96,7 +93,7 @@ public class NFHttpClientFactory {
 	    if(c != null) {
 	        c.getConnectionManager().shutdown();
 	        namedClientMap.remove(name);
-	        Monitors.unregisterObject("HttpClient_" + name, c);
+	        Monitors.unregisterObject(name, c);
 	    }
 	}
 }