diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 6c161444e24..c828a73e17d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -1020,6 +1020,24 @@ public class CrudIT extends ESRestHighLevelClientTestCase { } } + 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"; diff --git a/docs/reference/migration/migrate_7_4.asciidoc b/docs/reference/migration/migrate_7_4.asciidoc index 92f04767aff..ebfca7d25c1 100644 --- a/docs/reference/migration/migrate_7_4.asciidoc +++ b/docs/reference/migration/migrate_7_4.asciidoc @@ -56,3 +56,15 @@ breaking change was made necessary by https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/[AWS's announcement] that the path-style access pattern is deprecated and will be unsupported on buckets created after September 30th 2020. + +[float] +[[breaking_80_http_changes]] +=== HTTP changes + +[float] +==== Changes to Encoding Plus Signs in URLs + +Starting in version 7.4, 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. \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/rest/RestUtils.java b/server/src/main/java/org/elasticsearch/rest/RestUtils.java index e2be316cf92..827174743f7 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestUtils.java +++ b/server/src/main/java/org/elasticsearch/rest/RestUtils.java @@ -19,6 +19,7 @@ package org.elasticsearch.rest; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Strings; import org.elasticsearch.common.path.PathTrie; @@ -30,6 +31,11 @@ import java.util.regex.Pattern; 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) { @@ -55,7 +61,7 @@ public class RestUtils { 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 == ';') { @@ -63,9 +69,9 @@ public class RestUtils { // 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; @@ -74,15 +80,19 @@ public class RestUtils { 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 params, String name, String value) { params.put(name, value); } @@ -90,7 +100,7 @@ public class RestUtils { /** * Decodes a bit of an URL encoded by a browser. *

- * 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). @@ -100,7 +110,7 @@ public class RestUtils { * 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); } /** @@ -119,52 +129,50 @@ public class RestUtils { * 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) { diff --git a/x-pack/plugin/ml/qa/basic-multi-node/src/test/java/org/elasticsearch/xpack/ml/integration/MlBasicMultiNodeIT.java b/x-pack/plugin/ml/qa/basic-multi-node/src/test/java/org/elasticsearch/xpack/ml/integration/MlBasicMultiNodeIT.java index 31b43eb0555..51e77bf97ae 100644 --- a/x-pack/plugin/ml/qa/basic-multi-node/src/test/java/org/elasticsearch/xpack/ml/integration/MlBasicMultiNodeIT.java +++ b/x-pack/plugin/ml/qa/basic-multi-node/src/test/java/org/elasticsearch/xpack/ml/integration/MlBasicMultiNodeIT.java @@ -14,9 +14,9 @@ import org.elasticsearch.common.Strings; 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; @@ -303,7 +303,7 @@ public class MlBasicMultiNodeIT extends ESRestTestCase { } 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); }