Skip to content

Commit

Permalink
[TEST] Work around URI encode limitations in RestClient
Browse files Browse the repository at this point in the history
We've been relying on URI for url encoding, but it turns out it has some problems. For instance '+' stays as is while it should be encoded to `%2B`. If we go and manually encode query params we have to be careful though not to run into double encoding ('+'=>'%2B'=>'%252B'). The applied solution relies on URI encoding for the url path, but manual url encoding for the query parameters. We prevent URI from double encoding query params by using its single argument constructor that leaves everything as is.

We can also revert back the expression script REST test that revealed this to its original content (which contains an addition).

Closes elastic#9769
Closes elastic#9946
  • Loading branch information
javanna committed Mar 3, 2015
1 parent e559471 commit 45ce7a1
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 13 deletions.
4 changes: 2 additions & 2 deletions rest-api-spec/test/script/30_expressions.yaml
Expand Up @@ -22,6 +22,6 @@ setup:
---
"Expressions scripting test":

- do: { search: { body: { script_fields : { my_field : { lang: expression, script: 'doc["age"].value' } } } } }
- match: { hits.hits.0.fields.my_field.0: 23.0 }
- do: { search: { body: { script_fields : { my_field : { lang: expression, script: 'doc["age"].value + 19' } } } } }
- match: { hits.hits.0.fields.my_field.0: 42.0 }

Expand Up @@ -31,8 +31,10 @@
import org.elasticsearch.http.HttpServerTransport;

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLEncoder;
import java.nio.charset.Charset;
import java.util.Map;

Expand Down Expand Up @@ -89,8 +91,13 @@ public HttpRequestBuilder path(String path) {
}

public HttpRequestBuilder addParam(String name, String value) {
this.params.put(name, value);
return this;
try {
//manually url encode params, since URI does it only partially (e.g. '+' stays as is)
this.params.put(name, URLEncoder.encode(value, "utf-8"));
return this;
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
}

public HttpRequestBuilder addHeaders(Headers headers) {
Expand Down Expand Up @@ -173,16 +180,18 @@ private HttpUriRequest buildRequest() {
}

private URI buildUri() {
String query;
if (params.size() == 0) {
query = null;
} else {
query = Joiner.on('&').withKeyValueSeparator("=").join(params);
}
try {
return new URI(protocol, null, host, port, path, query, null);
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
//url encode rules for path and query params are different. We use URI to encode the path, but we manually encode each query param through URLEncoder.
URI uri = new URI(protocol, null, host, port, path, null, null);
//String concatenation FTW. If we use the nicer multi argument URI constructor query parameters will get only partially encoded
//(e.g. '+' will stay as is) hence when trying to properly encode params manually they will end up double encoded (+ becomes %252B instead of %2B).
StringBuilder uriBuilder = new StringBuilder(protocol).append("://").append(host).append(":").append(port).append(uri.getRawPath());
if (params.size() > 0) {
uriBuilder.append("?").append(Joiner.on('&').withKeyValueSeparator("=").join(params));
}
return URI.create(uriBuilder.toString());
} catch(URISyntaxException e) {
throw new IllegalArgumentException("unable to build uri", e);
}
}

Expand Down

0 comments on commit 45ce7a1

Please sign in to comment.