Don't URL encode fragment identifiers (#937)

Fixes #936

This is a super simple fix to illustrate the issue.
If a different solution is preferred, then I'm open for suggestions of course.

Author: rejierd

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

@@ -43,6 +43,7 @@ public final class RequestTemplate implements Serializable {
   private final Map<String, QueryTemplate> queries = new LinkedHashMap<>();
   private final Map<String, HeaderTemplate> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
   private String target;
+  private String fragment;
   private boolean resolved = false;
   private UriTemplate uriTemplate;
   private HttpMethod method;
@@ -70,13 +71,15 @@ public RequestTemplate() {
    * @param collectionFormat when expanding collection based variables.
    */
   private RequestTemplate(String target,
+      String fragment,
       UriTemplate uriTemplate,
       HttpMethod method,
       Charset charset,
       Request.Body body,
       boolean decodeSlash,
       CollectionFormat collectionFormat) {
     this.target = target;
+    this.fragment = fragment;
     this.uriTemplate = uriTemplate;
     this.method = method;
     this.charset = charset;
@@ -94,7 +97,8 @@ private RequestTemplate(String target,
    */
   public static RequestTemplate from(RequestTemplate requestTemplate) {
     RequestTemplate template =
-        new RequestTemplate(requestTemplate.target, requestTemplate.uriTemplate,
+        new RequestTemplate(requestTemplate.target, requestTemplate.fragment,
+            requestTemplate.uriTemplate,
             requestTemplate.method, requestTemplate.charset,
             requestTemplate.body, requestTemplate.decodeSlash, requestTemplate.collectionFormat);
 
@@ -118,6 +122,7 @@ public static RequestTemplate from(RequestTemplate requestTemplate) {
   public RequestTemplate(RequestTemplate toCopy) {
     checkNotNull(toCopy, "toCopy");
     this.target = toCopy.target;
+    this.fragment = toCopy.fragment;
     this.method = toCopy.method;
     this.queries.putAll(toCopy.queries);
     this.headers.putAll(toCopy.headers);
@@ -421,6 +426,12 @@ public RequestTemplate uri(String uri, boolean append) {
       uri = uri.substring(0, queryMatcher.start());
     }
 
+    int fragmentIndex = uri.indexOf('#');
+    if (fragmentIndex > -1) {
+      fragment = uri.substring(fragmentIndex);
+      uri = uri.substring(0, fragmentIndex);
+    }
+
     /* replace the uri template */
     if (append && this.uriTemplate != null) {
       this.uriTemplate = UriTemplate.append(this.uriTemplate, uri);
@@ -462,6 +473,9 @@ public RequestTemplate target(String target) {
 
       /* strip the query string */
       this.target = targetUri.getScheme() + "://" + targetUri.getAuthority() + targetUri.getPath();
+      if (targetUri.getFragment() != null) {
+        this.fragment = "#" + targetUri.getFragment();
+      }
     } 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);
@@ -482,6 +496,9 @@ public String url() {
     if (!this.queries.isEmpty()) {
       url.append(this.queryLine());
     }
+    if (fragment != null) {
+      url.append(fragment);
+    }
 
     return url.toString();
   }

core/src/test/java/feign/RequestTemplateTest.java

@@ -29,6 +29,8 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import feign.Request.HttpMethod;
+import feign.template.UriUtils;
 
 public class RequestTemplateTest {
 
@@ -431,4 +433,24 @@ public void shouldNotInsertHeadersImmutableMap() {
 
     template.headers().put("key2", Collections.singletonList("other value"));
   }
+
+  @Test
+  public void fragmentShouldNotBeEncodedInUri() {
+    RequestTemplate template = new RequestTemplate()
+        .method(HttpMethod.GET)
+        .uri("/path#fragment")
+        .queries(mapOf("key1", Collections.singletonList("value1")));
+
+    assertThat(template.url()).isEqualTo("/path?key1=value1#fragment");
+  }
+
+  @Test
+  public void fragmentShouldNotBeEncodedInTarget() {
+    RequestTemplate template = new RequestTemplate()
+        .method(HttpMethod.GET)
+        .target("https://example.com/path#fragment")
+        .queries(mapOf("key1", Collections.singletonList("value1")));
+
+    assertThat(template.url()).isEqualTo("https://example.com/path?key1=value1#fragment");
+  }
 }