From 0ab3cbe3a3b15a154c79138c6d0e6f8dfa888876 Mon Sep 17 00:00:00 2001 From: Alexander Lin Date: Mon, 19 Dec 2016 12:56:09 -0800 Subject: [PATCH] Adds percent-encoding for Location headers (#21057) This should cause unicode elements in the location header to be percent-encoded, instead of being left alone. Closes #21016 --- .../action/DocWriteResponse.java | 9 ++++-- .../rest/action/document/RestIndexAction.java | 10 +++++- .../action/document/RestUpdateAction.java | 10 +++++- .../action/DocWriteResponseTests.java | 32 ++++++++++++++++++- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java b/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java index aef99494d92..f96dfcf0f7c 100644 --- a/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java +++ b/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java @@ -35,6 +35,8 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.rest.RestStatus; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.util.Locale; /** @@ -186,8 +188,9 @@ public abstract class DocWriteResponse extends ReplicationResponse implements Wr /** * Gets the location of the written document as a string suitable for a {@code Location} header. * @param routing any routing used in the request. If null the location doesn't include routing information. + * */ - public String getLocation(@Nullable String routing) { + public String getLocation(@Nullable String routing) throws URISyntaxException { // Absolute path for the location of the document. This should be allowed as of HTTP/1.1: // https://tools.ietf.org/html/rfc7231#section-7.1.2 String index = getIndex(); @@ -205,7 +208,9 @@ public abstract class DocWriteResponse extends ReplicationResponse implements Wr if (routing != null) { location.append(routingStart).append(routing); } - return location.toString(); + + URI uri = new URI(location.toString()); + return uri.toASCIIString(); } @Override diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index 3880ec6ca9e..43739976ba0 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -32,6 +32,7 @@ import org.elasticsearch.rest.action.RestActions; import org.elasticsearch.rest.action.RestStatusToXContentListener; import java.io.IOException; +import java.net.URISyntaxException; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; @@ -82,7 +83,14 @@ public class RestIndexAction extends BaseRestHandler { } return channel -> - client.index(indexRequest, new RestStatusToXContentListener<>(channel, r -> r.getLocation(indexRequest.routing()))); + client.index(indexRequest, new RestStatusToXContentListener<>(channel, r -> { + try { + return r.getLocation(indexRequest.routing()); + } catch (URISyntaxException ex) { + logger.warn("Location string is not a valid URI.", ex); + return null; + } + })); } } diff --git a/core/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java b/core/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java index ade0200215a..57039bdbe95 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java @@ -37,6 +37,7 @@ import org.elasticsearch.rest.action.RestStatusToXContentListener; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import java.io.IOException; +import java.net.URISyntaxException; import static org.elasticsearch.rest.RestRequest.Method.POST; @@ -99,6 +100,13 @@ public class RestUpdateAction extends BaseRestHandler { }); return channel -> - client.update(updateRequest, new RestStatusToXContentListener<>(channel, r -> r.getLocation(updateRequest.routing()))); + client.update(updateRequest, new RestStatusToXContentListener<>(channel, r -> { + try { + return r.getLocation(updateRequest.routing()); + } catch (URISyntaxException ex) { + logger.warn("Location string is not a valid URI.", ex); + return null; + } + })); } } diff --git a/core/src/test/java/org/elasticsearch/action/DocWriteResponseTests.java b/core/src/test/java/org/elasticsearch/action/DocWriteResponseTests.java index 017ba128d50..0f6332006d9 100644 --- a/core/src/test/java/org/elasticsearch/action/DocWriteResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/DocWriteResponseTests.java @@ -30,13 +30,14 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; import java.io.IOException; +import java.net.URISyntaxException; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.not; public class DocWriteResponseTests extends ESTestCase { - public void testGetLocation() { + public void testGetLocation() throws URISyntaxException { DocWriteResponse response = new DocWriteResponse( new ShardId("index", "uuid", 0), @@ -51,6 +52,35 @@ public class DocWriteResponseTests extends ESTestCase { assertEquals("/index/type/id?routing=test_routing", response.getLocation("test_routing")); } + public void testGetLocationNonAscii() throws URISyntaxException { + DocWriteResponse response = + new DocWriteResponse( + new ShardId("index", "uuid", 0), + "type", + "❤", + SequenceNumbersService.UNASSIGNED_SEQ_NO, + 0, + Result.CREATED) { + }; + assertEquals("/index/type/%E2%9D%A4", response.getLocation(null)); + assertEquals("/index/type/%E2%9D%A4?routing=%C3%A4", response.getLocation("%C3%A4")); + } + + public void testInvalidGetLocation() { + String invalidPath = "!^*$(@!^!#@"; + DocWriteResponse invalid = + new DocWriteResponse( + new ShardId("index", "uuid", 0), + "type", + invalidPath, + SequenceNumbersService.UNASSIGNED_SEQ_NO, + 0, + Result.CREATED) { + }; + Throwable exception = expectThrows(URISyntaxException.class, () -> invalid.getLocation(null)); + assertTrue(exception.getMessage().contains(invalidPath)); + } + /** * Tests that {@link DocWriteResponse#toXContent(XContentBuilder, ToXContent.Params)} doesn't include {@code forced_refresh} unless it * is true. We can't assert this in the yaml tests because "not found" is also "false" there....