Skip to content

Removed decoding from Body Template Expansion #931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 8, 2019
Merged

Removed decoding from Body Template Expansion #931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94949cc
Removed decoding from Body Template Expansion
kdavisk6 Mar 27, 2019
48287e5
Replaced String with String Builder when parsing Body Template JSON
kdavisk6 Apr 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion core/src/main/java/feign/template/BodyTemplate.java
View file
Open in desktop
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,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;
}


}
35 changes: 23 additions & 12 deletions core/src/test/java/feign/RequestTemplateTest.java
View file
Open in desktop
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