Skip to content

Commit

Permalink
Removed decoding from Body Template Expansion
Browse files Browse the repository at this point in the history
Fixes OpenFeign#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.
  • Loading branch information
kdavisk6 committed Mar 27, 2019
1 parent 10a4ead commit 94949cc
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 13 deletions.
22 changes: 21 additions & 1 deletion core/src/main/java/feign/template/BodyTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -35,10 +41,24 @@ 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 */
expanded = expanded.substring(
expanded.indexOf(JSON_TOKEN_START_ENCODED) + JSON_TOKEN_START_ENCODED.length());
expanded = JSON_TOKEN_START + expanded;
expanded = expanded.substring(0, expanded.lastIndexOf(JSON_TOKEN_END_ENCODED));
expanded += JSON_TOKEN_END;
}
return expanded;
}


}
35 changes: 23 additions & 12 deletions core/src/test/java/feign/RequestTemplateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 94949cc

Please sign in to comment.