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

HLRC: Fix '+' Not Correctly Encoded in GET Req. #33164

Merged
merged 24 commits into from
Jul 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d607643
HLRC: Fix '+' Not Correctly Encoded in GET Req.
original-brownbear Aug 27, 2018
127aa1c
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Aug 27, 2018
f3e9208
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Aug 27, 2018
c16be99
Remove server side handling of + as ' '
original-brownbear Aug 27, 2018
ce12e8d
fix test
original-brownbear Aug 27, 2018
e9862d9
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Aug 27, 2018
906d51e
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Sep 3, 2018
9da77eb
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Nov 25, 2018
33ac4c8
CR: Add system property for decoding + as space or not
original-brownbear Nov 25, 2018
88661c1
fix typo
original-brownbear Nov 25, 2018
1d59ba8
hack around encoding asymetry in saml
original-brownbear Nov 25, 2018
20ee2f8
Merge remote-tracking branch 'elastic' into 33077
original-brownbear Nov 26, 2018
4c65f6b
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Jan 5, 2019
9b7b010
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear May 29, 2019
d32bee1
fix test
original-brownbear May 29, 2019
1704338
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear May 30, 2019
e4397d1
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Jun 23, 2019
0f8be02
CR: Add deprecation/breaking changes note
original-brownbear Jun 23, 2019
c792bda
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Jun 23, 2019
357e801
Keep encoding + as space in query strings
original-brownbear Jun 23, 2019
fbe3323
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Jun 24, 2019
d0f38a1
correctly encode jobid in test
original-brownbear Jun 24, 2019
d34ae83
Merge remote-tracking branch 'elastic/master' into 33077
original-brownbear Jun 24, 2019
94b2ba9
CR:revert SAML change
original-brownbear Jun 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,24 @@ public void testParamsEncode() throws IOException {
}
}

public void testGetIdWithPlusSign() throws Exception {
String id = "id+id";
{
IndexRequest indexRequest = new IndexRequest("index").id(id);
indexRequest.source("field", "value");
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);
assertEquals("index", indexResponse.getIndex());
assertEquals(id, indexResponse.getId());
}
{
GetRequest getRequest = new GetRequest("index").id(id);
GetResponse getResponse = highLevelClient().get(getRequest, RequestOptions.DEFAULT);
assertTrue(getResponse.isExists());
assertEquals("index", getResponse.getIndex());
assertEquals(id, getResponse.getId());
}
}

// Not entirely sure if _termvectors belongs to CRUD, and in the absence of a better place, will have it here
public void testTermvectors() throws IOException {
final String sourceIndex = "index1";
Expand Down
10 changes: 9 additions & 1 deletion docs/reference/migration/migrate_8_0/http.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,12 @@
==== Removal of old HTTP settings

The `http.tcp_no_delay` setting was deprecated in 7.x and has been removed in 8.0. It has been replaced by
`http.tcp.no_delay`.
`http.tcp.no_delay`.

[float]
==== Changes to Encoding Plus Signs in URLs

Starting in version 7.3, a `+` in a URL will be encoded as `%2B` by all REST API functionality. Prior versions handled a `+` as a single space.
If your application requires handling `+` as a single space you can return to the old behaviour by setting the system property
`es.rest.url_plus_as_space` to `true`. Note that this behaviour is deprecated and setting this system property to `true` will cease
to be supported in version 8.
56 changes: 32 additions & 24 deletions server/src/main/java/org/elasticsearch/rest/RestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.rest;

import org.elasticsearch.common.Booleans;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.path.PathTrie;

Expand All @@ -30,6 +31,11 @@

public class RestUtils {

/**
* Sets whether we decode a '+' in an url as a space or not.
*/
private static final boolean DECODE_PLUS_AS_SPACE = Booleans.parseBoolean(System.getProperty("es.rest.url_plus_as_space", "false"));

public static final PathTrie.Decoder REST_DECODER = new PathTrie.Decoder() {
@Override
public String decode(String value) {
Expand All @@ -55,17 +61,17 @@ public static void decodeQueryString(String s, int fromIndex, Map<String, String
c = s.charAt(i);
if (c == '=' && name == null) {
if (pos != i) {
name = decodeComponent(s.substring(pos, i));
name = decodeQueryStringParam(s.substring(pos, i));
}
pos = i + 1;
} else if (c == '&' || c == ';') {
if (name == null && pos != i) {
// We haven't seen an `=' so far but moved forward.
// Must be a param of the form '&a&' so add it with
// an empty value.
addParam(params, decodeComponent(s.substring(pos, i)), "");
addParam(params, decodeQueryStringParam(s.substring(pos, i)), "");
} else if (name != null) {
addParam(params, name, decodeComponent(s.substring(pos, i)));
addParam(params, name, decodeQueryStringParam(s.substring(pos, i)));
name = null;
}
pos = i + 1;
Expand All @@ -74,23 +80,27 @@ public static void decodeQueryString(String s, int fromIndex, Map<String, String

if (pos != i) { // Are there characters we haven't dealt with?
if (name == null) { // Yes and we haven't seen any `='.
addParam(params, decodeComponent(s.substring(pos, i)), "");
addParam(params, decodeQueryStringParam(s.substring(pos, i)), "");
} else { // Yes and this must be the last value.
addParam(params, name, decodeComponent(s.substring(pos, i)));
addParam(params, name, decodeQueryStringParam(s.substring(pos, i)));
}
} else if (name != null) { // Have we seen a name without value?
addParam(params, name, "");
}
}

private static String decodeQueryStringParam(final String s) {
return decodeComponent(s, StandardCharsets.UTF_8, true);
}

private static void addParam(Map<String, String> params, String name, String value) {
params.put(name, value);
}

/**
* Decodes a bit of an URL encoded by a browser.
* <p>
* This is equivalent to calling {@link #decodeComponent(String, Charset)}
* This is equivalent to calling {@link #decodeComponent(String, Charset, boolean)}
* with the UTF-8 charset (recommended to comply with RFC 3986, Section 2).
*
* @param s The string to decode (can be empty).
Expand All @@ -100,7 +110,7 @@ private static void addParam(Map<String, String> params, String name, String val
* escape sequence.
*/
public static String decodeComponent(final String s) {
return decodeComponent(s, StandardCharsets.UTF_8);
return decodeComponent(s, StandardCharsets.UTF_8, DECODE_PLUS_AS_SPACE);
}

/**
Expand All @@ -119,52 +129,50 @@ public static String decodeComponent(final String s) {
* Actually this function doesn't allocate any memory if there's nothing
* to decode, the argument itself is returned.
*
* @param s The string to decode (can be empty).
* @param charset The charset to use to decode the string (should really
* be {@link StandardCharsets#UTF_8}.
* @param s The string to decode (can be empty).
* @param charset The charset to use to decode the string (should really
* be {@link StandardCharsets#UTF_8}.
* @param plusAsSpace Whether to decode a {@code '+'} to a single space {@code ' '}
* @return The decoded string, or {@code s} if there's nothing to decode.
* If the string to decode is {@code null}, returns an empty string.
* @throws IllegalArgumentException if the string contains a malformed
* escape sequence.
*/
public static String decodeComponent(final String s, final Charset charset) {
private static String decodeComponent(final String s, final Charset charset, boolean plusAsSpace) {
if (s == null) {
return "";
}
final int size = s.length();
if (!decodingNeeded(s, size)) {
if (!decodingNeeded(s, size, plusAsSpace)) {
return s;
}
final byte[] buf = new byte[size];
int pos = decode(s, size, buf);
int pos = decode(s, size, buf, plusAsSpace);
return new String(buf, 0, pos, charset);
}

@SuppressWarnings("fallthrough")
private static boolean decodingNeeded(String s, int size) {
private static boolean decodingNeeded(String s, int size, boolean plusAsSpace) {
boolean decodingNeeded = false;
for (int i = 0; i < size; i++) {
final char c = s.charAt(i);
switch (c) {
case '%':
i++; // We can skip at least one char, e.g. `%%'.
// Fall through.
case '+':
decodingNeeded = true;
break;
if (c == '%') {
i++; // We can skip at least one char, e.g. `%%'.
decodingNeeded = true;
} else if (plusAsSpace && c == '+') {
decodingNeeded = true;
}
}
return decodingNeeded;
}

@SuppressWarnings("fallthrough")
private static int decode(String s, int size, byte[] buf) {
private static int decode(String s, int size, byte[] buf, boolean plusAsSpace) {
int pos = 0; // position in `buf'.
for (int i = 0; i < size; i++) {
char c = s.charAt(i);
switch (c) {
case '+':
buf[pos++] = ' '; // "+" -> " "
buf[pos++] = (byte) (plusAsSpace ? ' ' : '+'); // "+" -> " "
break;
case '%':
if (i == size - 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.elasticsearch.xpack.ml.MachineLearning;
import org.yaml.snakeyaml.util.UriEncoder;

import java.io.IOException;
import java.net.URLEncoder;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -303,7 +303,7 @@ private Response createFarequoteJob(String jobId) throws Exception {
}
xContentBuilder.endObject();

Request request = new Request("PUT", MachineLearning.BASE_PATH + "anomaly_detectors/" + URLEncoder.encode(jobId, "UTF-8"));
Request request = new Request("PUT", MachineLearning.BASE_PATH + "anomaly_detectors/" + UriEncoder.encode(jobId));
request.setJsonEntity(Strings.toString(xContentBuilder));
return client().performRequest(request);
}
Expand Down