Skip to content
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.

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

Adding URI segment specific encoding #882

Merged
merged 4 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ See [Advanced Usage](#advanced-usage) for more examples.
>
> `@RequestLine` and `@QueryMap` templates do not encode slash `/` characters by default. To change this behavior, set the `decodeSlash` property on the `@RequestLine` to `false`.

> **What about plus? `+`**
>
> Per the URI specification, a `+` sign is allowed in both the path and query segments of a URI, however, handling of
> the symbol on the query can be inconsistent. In some legacy systems, the `+` is equivalent to the a space. Feign takes the approach of modern systems, where a
> `+` symbol should not represent a space and is explicitly encoded as `%2B` when found on a query string.
>
> If you wish to use `+` as a space, then use the literal ` ` character or encode the value directly as `%20`

##### Custom Expansion

The `@Param` annotation has an optional property `expander` allowing for complete control over the individual parameter's expansion.
Expand Down
35 changes: 9 additions & 26 deletions core/src/main/java/feign/template/Expressions.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2012-2018 The Feign Authors
* Copyright 2012-2019 The Feign Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -11,10 +11,10 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package feign.template;

import feign.Util;
import feign.template.UriUtils.FragmentType;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -29,11 +29,10 @@ public final class Expressions {

static {
expressions = new LinkedHashMap<>();
expressions.put(Pattern.compile("\\+(\\w[-\\w.]*[ ]*)(:(.+))?"), ReservedExpression.class);
expressions.put(Pattern.compile("(\\w[-\\w.]*[ ]*)(:(.+))?"), SimpleExpression.class);
}

public static Expression create(final String value) {
public static Expression create(final String value, final FragmentType type) {

/* remove the start and end braces */
final String expression = stripBraces(value);
Expand All @@ -54,7 +53,6 @@ public static Expression create(final String value) {

Entry<Pattern, Class<? extends Expression>> matchedExpression = matchedExpressionEntry.get();
Pattern expressionPattern = matchedExpression.getKey();
Class<? extends Expression> expressionType = matchedExpression.getValue();

/* create a new regular expression matcher for the expression */
String variableName = null;
Expand All @@ -69,7 +67,7 @@ public static Expression create(final String value) {
}
}

return new SimpleExpression(variableName, variablePattern);
return new SimpleExpression(variableName, variablePattern, type);
}

private static String stripBraces(String expression) {
Expand All @@ -82,36 +80,21 @@ private static String stripBraces(String expression) {
return expression;
}

/**
* Expression that does not encode reserved characters. This expression adheres to RFC 6570
* <a href="https://tools.ietf.org/html/rfc6570#section-3.2.3">Reserved Expansion (Level 2)</a>
* specification.
*/
public static class ReservedExpression extends SimpleExpression {
private final String RESERVED_CHARACTERS = ":/?#[]@!$&\'()*+,;=";

ReservedExpression(String expression, String pattern) {
super(expression, pattern);
}

@Override
String encode(Object value) {
return UriUtils.encodeReserved(value.toString(), RESERVED_CHARACTERS, Util.UTF_8);
}
}

/**
* Expression that adheres to Simple String Expansion as outlined in <a
* href="https://tools.ietf.org/html/rfc6570#section-3.2.2>Simple String Expansion (Level 1)</a>
*/
static class SimpleExpression extends Expression {

SimpleExpression(String expression, String pattern) {
private final FragmentType type;

SimpleExpression(String expression, String pattern, FragmentType type) {
super(expression, pattern);
this.type = type;
}

String encode(Object value) {
return UriUtils.encode(value.toString(), Util.UTF_8);
return UriUtils.encodeReserved(value.toString(), type, Util.UTF_8);
}

@Override
Expand Down
11 changes: 7 additions & 4 deletions core/src/main/java/feign/template/Template.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2012-2018 The Feign Authors
* Copyright 2012-2019 The Feign Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -13,6 +13,7 @@
*/
package feign.template;

import feign.template.UriUtils.FragmentType;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -80,9 +81,9 @@ public String expand(Map<String, ?> variables) {
Object value = variables.get(expression.getName());
if (value != null) {
String expanded = expression.expand(value, this.encode.isEncodingRequired());
if (!this.encodeSlash) {
if (this.encodeSlash) {
logger.fine("Explicit slash decoding specified, decoding all slashes in uri");
expanded = expanded.replaceAll("\\%2F", "/");
expanded = expanded.replaceAll("/", "%2F");
}
resolved.append(expanded);
} else {
Expand Down Expand Up @@ -200,7 +201,9 @@ private void parseFragment(String fragment, boolean query) {

if (chunk.startsWith("{")) {
/* it's an expression, defer encoding until resolution */
Expression expression = Expressions.create(chunk);
FragmentType type = (query) ? FragmentType.QUERY : FragmentType.PATH_SEGMENT;

Expression expression = Expressions.create(chunk, type);
if (expression == null) {
this.templateChunks.add(Literal.create(encode(chunk, query)));
} else {
Expand Down
Loading