No Longer prepend uri with slash if it is a query string (#889)

* No Longer prepend uri with slash if it is a query string

Fixes #887

Changes to target and uri parsing did not take into account
Empty Targets or URI provided methods.  In these scenarios,
the target contains the entire path and the uri will be the
query string only.  This change takes that into account and
no longer prepends a slash in these cases.

* Removed unnecessary import for Spring from Target Test

Author: kdavisk6

core/src/main/java/feign/RequestTemplate.java

@@ -21,6 +21,7 @@
 import feign.template.UriTemplate;
 import feign.template.UriUtils;
 import java.io.Serializable;
+import java.net.URI;
 import java.nio.charset.Charset;
 import java.util.*;
 import java.util.AbstractMap.SimpleImmutableEntry;
@@ -400,7 +401,10 @@ public RequestTemplate uri(String uri, boolean append) {
 
     if (uri == null) {
       uri = "/";
-    } else if ((!uri.isEmpty() && !uri.startsWith("/") && !uri.startsWith("{"))) {
+    } else if ((!uri.isEmpty()
+        && !uri.startsWith("/")
+        && !uri.startsWith("{")
+        && !uri.startsWith("?"))) {
       /* if the start of the url is a literal, it must begin with a slash. */
       uri = "/" + uri;
     }
@@ -448,20 +452,23 @@ public RequestTemplate target(String target) {
     if (target.endsWith("/")) {
       target = target.substring(0, target.length() - 1);
     }
-
-    /* no query strings allowed */
-    Matcher queryStringMatcher = QUERY_STRING_PATTERN.matcher(target);
-    if (queryStringMatcher.find()) {
-      /*
-       * target has a query string, we need to make sure that they are recorded as queries
-       */
-      int queryStart = queryStringMatcher.start();
-      this.extractQueryTemplates(target.substring(queryStart + 1), true);
+    try {
+      /* parse the target */
+      URI targetUri = URI.create(target);
+
+      if (Util.isNotBlank(targetUri.getRawQuery())) {
+        /*
+         * target has a query string, we need to make sure that they are recorded as queries
+         */
+        this.extractQueryTemplates(targetUri.getRawQuery(), true);
+      }
 
       /* strip the query string */
-      target = target.substring(0, queryStart);
+      this.target = targetUri.getScheme() + "://" + targetUri.getAuthority() + targetUri.getPath();
+    } catch (IllegalArgumentException iae) {
+      /* the uri provided is not a valid one, we can't continue */
+      throw new IllegalArgumentException("Target is not a valid URI.", iae);
     }
-    this.target = target;
     return this;
   }
 

core/src/test/java/feign/LoggerTest.java

@@ -381,7 +381,7 @@ public Retryer clone() {
                       return this;
                     }
                   })
-              .target(SendsStuff.class, "http://sna%fu.abc");
+              .target(SendsStuff.class, "http://sna%25fu.abc");
 
       thrown.expect(FeignException.class);
 

core/src/test/java/feign/TargetTest.java

@@ -16,6 +16,7 @@
 import static feign.assertj.MockWebServerAssertions.assertThat;
 
 import feign.Target.HardCodedTarget;
+import java.net.URI;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import org.junit.Rule;
@@ -70,4 +71,39 @@ public Request apply(RequestTemplate input) {
 
     assertThat(server.takeRequest()).hasPath("/default/slash/foo?query=slash/bar");
   }
+
+  interface UriTarget {
+
+    @RequestLine("GET")
+    Response get(URI uri);
+  }
+
+  @Test
+  public void emptyTarget() throws InterruptedException {
+    server.enqueue(new MockResponse());
+
+    UriTarget uriTarget = Feign.builder().target(Target.EmptyTarget.create(UriTarget.class));
+
+    String host = server.getHostName();
+    int port = server.getPort();
+
+    uriTarget.get(URI.create("http://" + host + ":" + port + "/path?query=param"));
+
+    assertThat(server.takeRequest()).hasPath("/path?query=param").hasQueryParams("query=param");
+  }
+
+  @Test
+  public void hardCodedTargetWithURI() throws InterruptedException {
+    server.enqueue(new MockResponse());
+
+    String host = server.getHostName();
+    int port = server.getPort();
+    String base = "http://" + host + ":" + port;
+
+    UriTarget uriTarget = Feign.builder().target(UriTarget.class, base);
+
+    uriTarget.get(URI.create("http://" + host + ":" + port + "/path?query=param"));
+
+    assertThat(server.takeRequest()).hasPath("/path?query=param").hasQueryParams("query=param");
+  }
 }