Removed decoding from Body Template Expansion (#931)

Removed decoding from Body Template Expansion

Fixes #916

In certain cases, a Body Template will contain a JSON payload.  To
support this we are asking users to pct-encode the beginning and the
end of the JSON object when providing it to the RequestLine so we don't
reject it as an expression.  Doing this requires that the we decode those
markers before submitting the request.

This change updates that logic to only decode the first and last characters
only and not decode the entire payload, since Body values don't require
any type of encoding.

core/src/main/java/feign/template/BodyTemplate.java

@@ -23,6 +23,12 @@
  */
 public final class BodyTemplate extends Template {
 
+  private static final String JSON_TOKEN_START = "{";
+  private static final String JSON_TOKEN_END = "}";
+  private static final String JSON_TOKEN_START_ENCODED = "%7B";
+  private static final String JSON_TOKEN_END_ENCODED = "%7D";
+  private boolean json = false;
+
   /**
    * Create a new Body Template.
    *
@@ -35,10 +41,26 @@ public static BodyTemplate create(String template) {
 
   private BodyTemplate(String value, Charset charset) {
     super(value, ExpansionOptions.ALLOW_UNRESOLVED, EncodingOptions.NOT_REQUIRED, false, charset);
+    if (value.startsWith(JSON_TOKEN_START_ENCODED) && value.endsWith(JSON_TOKEN_END_ENCODED)) {
+      this.json = true;
+    }
   }
 
   @Override
   public String expand(Map<String, ?> variables) {
-    return UriUtils.decode(super.expand(variables), Util.UTF_8);
+    String expanded = super.expand(variables);
+    if (this.json) {
+      /* decode only the first and last character */
+      StringBuilder sb = new StringBuilder();
+      sb.append(JSON_TOKEN_START);
+      sb.append(expanded,
+          expanded.indexOf(JSON_TOKEN_START_ENCODED) + JSON_TOKEN_START_ENCODED.length(),
+          expanded.lastIndexOf(JSON_TOKEN_END_ENCODED));
+      sb.append(JSON_TOKEN_END);
+      return sb.toString();
+    }
+    return expanded;
   }
+
+
 }

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

@@ -16,10 +16,16 @@
 import static feign.assertj.FeignAssertions.assertThat;
 import static java.util.Arrays.asList;
 import static org.assertj.core.data.MapEntry.entry;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import feign.Request.HttpMethod;
 import feign.template.UriUtils;
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -97,8 +103,7 @@ public void resolveTemplateWithParameterizedPathSkipsEncodingSlash() {
   public void resolveTemplateWithBinaryBody() {
     RequestTemplate template = new RequestTemplate().method(HttpMethod.GET)
         .uri("{zoneId}")
-        .body(new byte[] {7, 3, -3, -7}, null);
-
+        .body(Request.Body.encoded(new byte[] {7, 3, -3, -7}, null));
     template = template.resolve(mapOf("zoneId", "/hostedzone/Z1PA6795UKMFR9"));
 
     assertThat(template)
@@ -185,7 +190,9 @@ public void resolveTemplateWithHeaderWithEscapedCurlyBrace() {
         .hasHeaders(entry("Encoded", Collections.singletonList("{{{{dont_expand_me}}")));
   }
 
-  /** This ensures we don't mess up vnd types */
+  /**
+   * This ensures we don't mess up vnd types
+   */
   @Test
   public void resolveTemplateWithHeaderIncludingSpecialCharacters() {
     RequestTemplate template = new RequestTemplate().method(HttpMethod.GET)
@@ -244,9 +251,10 @@ public void insertHasQueryParams() {
   @Test
   public void resolveTemplateWithBodyTemplateSetsBodyAndContentLength() {
     RequestTemplate template = new RequestTemplate().method(HttpMethod.POST)
-        .bodyTemplate(
+        .body(Request.Body.bodyTemplate(
             "%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", " +
-                "\"password\": \"{password}\"%7D");
+                "\"password\": \"{password}\"%7D",
+            Util.UTF_8));
 
     template = template.resolve(
         mapOf(
@@ -259,14 +267,15 @@ public void resolveTemplateWithBodyTemplateSetsBodyAndContentLength() {
             "{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"}")
         .hasHeaders(
             entry("Content-Length",
-                Collections.singletonList(String.valueOf(template.body().length))));
+                Collections.singletonList(String.valueOf(template.requestBody().length()))));
   }
 
   @Test
   public void resolveTemplateWithBodyTemplateDoesNotDoubleDecode() {
     RequestTemplate template = new RequestTemplate().method(HttpMethod.POST)
-        .bodyTemplate(
-            "%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\": \"{password}\"%7D");
+        .body(Request.Body.bodyTemplate(
+            "%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\": \"{password}\"%7D",
+            Util.UTF_8));
 
     template = template.resolve(
         mapOf(
@@ -276,7 +285,7 @@ public void resolveTemplateWithBodyTemplateDoesNotDoubleDecode() {
 
     assertThat(template)
         .hasBody(
-            "{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"abc 123%d8\"}");
+            "{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"abc+123%25d8\"}");
   }
 
   @Test
@@ -353,7 +362,9 @@ public void encodeSlashTest() {
         .hasUrl("/api/%2F");
   }
 
-  /** Implementations have a bug if they pass junk as the http method. */
+  /**
+   * Implementations have a bug if they pass junk as the http method.
+   */
   @SuppressWarnings("deprecation")
   @Test
   public void uriStuffedIntoMethod() {