Browse Source

Fixes vnd header regression by changing Headers encoding

I mistakenly advised `@Headers` to follow the encoding rules of `@Body`.
This was a a mistake as in both cases, url encoding is a bad choice, if
the only goal is to prevent accidental variable expansion. For example,
url encoding interferes a lot with content, including messing with '+'
characters, such as exist in "Accept: application/vnd.github.v3+json"

This changes `@Headers` to only address the problem, which where a '{'
literal is desired in a header value. The solution offered here is to
simply repeat "{{" when you desire a '{' literal. For example, if your
header value needs to be literally "{{variable}}", you'd encode it as
"{{{{variable}}".

The impact of this change is limited to those who have already started
using v8.15, and a fast release will occur after merge to limit that.

See #326
Fixes #345
Closes #346
pull/403/head
Adrian Cole 9 years ago
parent
commit
30aacc3dbf
  1. 33
      core/src/main/java/feign/RequestTemplate.java
  2. 20
      core/src/test/java/feign/RequestTemplateTest.java

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

@ -59,7 +59,6 @@ public final class RequestTemplate implements Serializable { @@ -59,7 +59,6 @@ public final class RequestTemplate implements Serializable {
private boolean decodeSlash = true;
public RequestTemplate() {
}
/* Copy constructor. Use this when making templates. */
@ -127,9 +126,19 @@ public final class RequestTemplate implements Serializable { @@ -127,9 +126,19 @@ public final class RequestTemplate implements Serializable {
for (char c : template.toCharArray()) {
switch (c) {
case '{':
if (inVar) {
// '{{' is an escape: write the brace and don't interpret as a variable
builder.append("{");
inVar = false;
break;
}
inVar = true;
break;
case '}':
if (!inVar) { // then write the brace literally
builder.append('}');
break;
}
inVar = false;
String key = var.toString();
Object value = variables.get(var.toString());
@ -208,13 +217,11 @@ public final class RequestTemplate implements Serializable { @@ -208,13 +217,11 @@ public final class RequestTemplate implements Serializable {
}
url = new StringBuilder(resolvedUrl);
Map<String, Collection<String>>
resolvedHeaders =
new LinkedHashMap<String, Collection<String>>();
Map<String, Collection<String>> resolvedHeaders = new LinkedHashMap<String, Collection<String>>();
for (String field : headers.keySet()) {
Collection<String> resolvedValues = new ArrayList<String>();
for (String value : valuesOrEmpty(headers, field)) {
String resolved = urlDecode(expand(value, encoded));
String resolved = expand(value, unencoded);
resolvedValues.add(resolved);
}
resolvedHeaders.put(field, resolvedValues);
@ -283,7 +290,7 @@ public final class RequestTemplate implements Serializable { @@ -283,7 +290,7 @@ public final class RequestTemplate implements Serializable {
}
/**
* Replaces queries with the specified {@code configKey} with url decoded {@code values} supplied.
* Replaces queries with the specified {@code name} with url decoded {@code values} supplied.
* <br> When the {@code value} is {@code null}, all queries with the {@code configKey} are
* removed. <br> <br><br><b>relationship to JAXRS 2.0</b><br> <br> Like {@code WebTarget.query},
* except the values can be templatized. <br> ex. <br>
@ -291,29 +298,29 @@ public final class RequestTemplate implements Serializable { @@ -291,29 +298,29 @@ public final class RequestTemplate implements Serializable {
* template.query(&quot;Signature&quot;, &quot;{signature}&quot;);
* </pre>
*
* @param configKey the configKey of the query
* @param name the name of the query
* @param values can be a single null to imply removing all values. Else no values are expected
* to be null.
* @see #queries()
*/
public RequestTemplate query(String configKey, String... values) {
queries.remove(checkNotNull(configKey, "configKey"));
public RequestTemplate query(String name, String... values) {
queries.remove(checkNotNull(name, "name"));
if (values != null && values.length > 0 && values[0] != null) {
ArrayList<String> encoded = new ArrayList<String>();
for (String value : values) {
encoded.add(encodeIfNotVariable(value));
}
this.queries.put(encodeIfNotVariable(configKey), encoded);
this.queries.put(encodeIfNotVariable(name), encoded);
}
return this;
}
/* @see #query(String, String...) */
public RequestTemplate query(String configKey, Iterable<String> values) {
public RequestTemplate query(String name, Iterable<String> values) {
if (values != null) {
return query(configKey, toArray(values, String.class));
return query(name, toArray(values, String.class));
}
return query(configKey, (String[]) null);
return query(name, (String[]) null);
}
private String encodeIfNotVariable(String in) {

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

@ -149,14 +149,26 @@ public class RequestTemplateTest { @@ -149,14 +149,26 @@ public class RequestTemplateTest {
}
@Test
public void resolveTemplateWithHeaderWithURLEncodedElements() {
public void resolveTemplateWithHeaderWithEscapedCurlyBrace() {
RequestTemplate template = new RequestTemplate().method("GET")
.header("Encoded", "%7Bvar%7D");
.header("Encoded", "{{{{dont_expand_me}}");
template.resolve(mapOf("var", "1234"));
template.resolve(mapOf("dont_expand_me", "1234"));
assertThat(template)
.hasHeaders(entry("Encoded", asList("{var}")));
.hasHeaders(entry("Encoded", asList("{{dont_expand_me}}")));
}
/** This ensures we don't mess up vnd types */
@Test
public void resolveTemplateWithHeaderIncludingSpecialCharacters() {
RequestTemplate template = new RequestTemplate().method("GET")
.header("Accept", "application/vnd.github.v3+{type}");
template.resolve(mapOf("type", "json"));
assertThat(template)
.hasHeaders(entry("Accept", asList("application/vnd.github.v3+json")));
}
@Test

Loading…
Cancel
Save