From 57108c8575b39e7b6c0e7c2ca4bf20f03e6d0912 Mon Sep 17 00:00:00 2001 From: kimchy Date: Thu, 10 Feb 2011 03:18:01 +0200 Subject: [PATCH] REST API: Failure to index docs that have their ids URL encoded and contain `/`, closes #681. --- .../elasticsearch/common/path/PathTrie.java | 31 ++++++++++++++----- .../org/elasticsearch/http/HttpServer.java | 18 ++++++----- .../http/netty/NettyHttpRequest.java | 10 +++--- .../elasticsearch/rest/RestController.java | 18 ++++++----- .../org/elasticsearch/rest/RestRequest.java | 7 ++++- .../rest/support/AbstractRestRequest.java | 4 +++ .../elasticsearch/rest/support/RestUtils.java | 7 +++++ .../memcached/MemcachedRestRequest.java | 10 +++--- .../thrift/ThriftRestRequest.java | 10 +++--- 9 files changed, 78 insertions(+), 37 deletions(-) diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/common/path/PathTrie.java b/modules/elasticsearch/src/main/java/org/elasticsearch/common/path/PathTrie.java index 552c8cfd2e3..0ff9097ff40 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/common/path/PathTrie.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/common/path/PathTrie.java @@ -30,20 +30,37 @@ import static org.elasticsearch.common.collect.MapBuilder.*; * @author kimchy (Shay Banon) */ public class PathTrie { + + public static interface Decoder { + String decode(String value); + } + + public static final Decoder NO_DECODER = new Decoder() { + @Override public String decode(String value) { + return value; + } + }; + + private final Decoder decoder; private final TrieNode root; private final Pattern pattern; private T rootValue; public PathTrie() { - this("/", "*"); + this("/", "*", NO_DECODER); } - public PathTrie(String separator, String wildcard) { + public PathTrie(Decoder decoder) { + this("/", "*", decoder); + } + + public PathTrie(String separator, String wildcard, Decoder decoder) { + this.decoder = decoder; pattern = Pattern.compile(separator); root = new TrieNode(separator, null, null, wildcard); } - public static class TrieNode { + public class TrieNode { private transient String key; private transient T value; private boolean isWildcard; @@ -169,6 +186,10 @@ public class PathTrie { return res; } + + private void put(Map params, String key, String value) { + params.put(key, decoder.decode(value)); + } } public void insert(String path, T value) { @@ -204,8 +225,4 @@ public class PathTrie { } return root.retrieve(strings, index, params); } - - private static void put(Map params, String key, String value) { - params.put(key, value); - } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/http/HttpServer.java b/modules/elasticsearch/src/main/java/org/elasticsearch/http/HttpServer.java index 8ee1f0c8d77..1e4d5e2d720 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/http/HttpServer.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/http/HttpServer.java @@ -29,6 +29,7 @@ import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.StringRestResponse; import org.elasticsearch.rest.XContentThrowableRestResponse; +import org.elasticsearch.rest.support.RestUtils; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; @@ -48,12 +49,12 @@ public class HttpServer extends AbstractLifecycleComponent { private final TransportNodesInfoAction nodesInfoAction; - private final PathTrie getHandlers = new PathTrie(); - private final PathTrie postHandlers = new PathTrie(); - private final PathTrie putHandlers = new PathTrie(); - private final PathTrie deleteHandlers = new PathTrie(); - private final PathTrie headHandlers = new PathTrie(); - private final PathTrie optionsHandlers = new PathTrie(); + private final PathTrie getHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie postHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie putHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie deleteHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie headHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie optionsHandlers = new PathTrie(RestUtils.REST_DECODER); @Inject public HttpServer(Settings settings, HttpServerTransport transport, ThreadPool threadPool, RestController restController, TransportNodesInfoAction nodesInfoAction) { @@ -166,6 +167,9 @@ public class HttpServer extends AbstractLifecycleComponent { } private String getPath(HttpRequest request) { - return request.path(); + // we use rawPath since we don't want to decode it while processing the path resolution + // so we can handle things like: + // my_index/my_type/http%3A%2F%2Fwww.google.com + return request.rawPath(); } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/http/netty/NettyHttpRequest.java b/modules/elasticsearch/src/main/java/org/elasticsearch/http/netty/NettyHttpRequest.java index 5886995e7c8..2fcef7f23a0 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/http/netty/NettyHttpRequest.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/http/netty/NettyHttpRequest.java @@ -39,7 +39,7 @@ public class NettyHttpRequest extends AbstractRestRequest implements HttpRequest private final Map params; - private final String path; + private final String rawPath; private byte[] cachedData; @@ -50,9 +50,9 @@ public class NettyHttpRequest extends AbstractRestRequest implements HttpRequest String uri = request.getUri(); int pathEndPos = uri.indexOf('?'); if (pathEndPos < 0) { - this.path = RestUtils.decodeComponent(uri); + this.rawPath = uri; } else { - this.path = RestUtils.decodeComponent(uri.substring(0, pathEndPos)); + this.rawPath = uri.substring(0, pathEndPos); RestUtils.decodeQueryString(uri, pathEndPos + 1, params); } } @@ -86,8 +86,8 @@ public class NettyHttpRequest extends AbstractRestRequest implements HttpRequest return request.getUri(); } - @Override public String path() { - return path; + @Override public String rawPath() { + return rawPath; } @Override public Map params() { diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestController.java b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestController.java index 96ca56023ff..fab070c9db3 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestController.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestController.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.path.PathTrie; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.support.RestUtils; import java.io.IOException; @@ -33,12 +34,12 @@ import java.io.IOException; */ public class RestController extends AbstractLifecycleComponent { - private final PathTrie getHandlers = new PathTrie(); - private final PathTrie postHandlers = new PathTrie(); - private final PathTrie putHandlers = new PathTrie(); - private final PathTrie deleteHandlers = new PathTrie(); - private final PathTrie headHandlers = new PathTrie(); - private final PathTrie optionsHandlers = new PathTrie(); + private final PathTrie getHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie postHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie putHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie deleteHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie headHandlers = new PathTrie(RestUtils.REST_DECODER); + private final PathTrie optionsHandlers = new PathTrie(RestUtils.REST_DECODER); @Inject public RestController(Settings settings) { super(settings); @@ -116,6 +117,9 @@ public class RestController extends AbstractLifecycleComponent { } private String getPath(RestRequest request) { - return request.path(); + // we use rawPath since we don't want to decode it while processing the path resolution + // so we can handle things like: + // my_index/my_type/http%3A%2F%2Fwww.google.com + return request.rawPath(); } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestRequest.java b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestRequest.java index 5ebfe007aac..37bbbfb4df8 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -43,7 +43,12 @@ public interface RestRequest extends ToXContent.Params { String uri(); /** - * The path part of the URI (without the query string). + * The non decoded, raw path provided. + */ + String rawPath(); + + /** + * The path part of the URI (without the query string), decoded. */ String path(); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/AbstractRestRequest.java b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/AbstractRestRequest.java index 7167370f4a5..be124faa5d8 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/AbstractRestRequest.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/AbstractRestRequest.java @@ -37,6 +37,10 @@ public abstract class AbstractRestRequest implements RestRequest { private static final Pattern commaPattern = Pattern.compile(","); + @Override public final String path() { + return RestUtils.decodeComponent(rawPath()); + } + @Override public float paramAsFloat(String key, float defaultValue) { String sValue = param(key); if (sValue == null) { diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/RestUtils.java b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/RestUtils.java index 2959143829e..1ac1e0303c8 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/RestUtils.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/rest/support/RestUtils.java @@ -20,6 +20,7 @@ package org.elasticsearch.rest.support; import org.elasticsearch.common.base.Charsets; +import org.elasticsearch.common.path.PathTrie; import java.nio.charset.Charset; import java.util.Map; @@ -29,6 +30,12 @@ import java.util.Map; */ public class RestUtils { + public static PathTrie.Decoder REST_DECODER = new PathTrie.Decoder() { + @Override public String decode(String value) { + return RestUtils.decodeComponent(value); + } + }; + public static void decodeQueryString(String s, int fromIndex, Map params) { if (fromIndex < 0) { return; diff --git a/plugins/transport/memcached/src/main/java/org/elasticsearch/memcached/MemcachedRestRequest.java b/plugins/transport/memcached/src/main/java/org/elasticsearch/memcached/MemcachedRestRequest.java index eed84dcd565..b24cb604bde 100644 --- a/plugins/transport/memcached/src/main/java/org/elasticsearch/memcached/MemcachedRestRequest.java +++ b/plugins/transport/memcached/src/main/java/org/elasticsearch/memcached/MemcachedRestRequest.java @@ -45,7 +45,7 @@ public class MemcachedRestRequest extends AbstractRestRequest { private final Map params; - private final String path; + private final String rawPath; private byte[] data; @@ -62,9 +62,9 @@ public class MemcachedRestRequest extends AbstractRestRequest { this.params = new HashMap(); int pathEndPos = uri.indexOf('?'); if (pathEndPos < 0) { - this.path = uri; + this.rawPath = uri; } else { - this.path = uri.substring(0, pathEndPos); + this.rawPath = uri.substring(0, pathEndPos); RestUtils.decodeQueryString(uri, pathEndPos + 1, params); } } @@ -77,8 +77,8 @@ public class MemcachedRestRequest extends AbstractRestRequest { return this.uri; } - @Override public String path() { - return this.path; + @Override public String rawPath() { + return this.rawPath; } public byte[] getUriBytes() { diff --git a/plugins/transport/thrift/src/main/java/org/elasticsearch/thrift/ThriftRestRequest.java b/plugins/transport/thrift/src/main/java/org/elasticsearch/thrift/ThriftRestRequest.java index f71bdfc5f33..2ccc9c5baf3 100644 --- a/plugins/transport/thrift/src/main/java/org/elasticsearch/thrift/ThriftRestRequest.java +++ b/plugins/transport/thrift/src/main/java/org/elasticsearch/thrift/ThriftRestRequest.java @@ -36,7 +36,7 @@ public class ThriftRestRequest extends AbstractRestRequest implements org.elasti private final org.elasticsearch.thrift.RestRequest request; - private final String path; + private final String rawPath; private final Map params; @@ -46,9 +46,9 @@ public class ThriftRestRequest extends AbstractRestRequest implements org.elasti int pathEndPos = request.getUri().indexOf('?'); if (pathEndPos < 0) { - this.path = request.getUri(); + this.rawPath = request.getUri(); } else { - this.path = request.getUri().substring(0, pathEndPos); + this.rawPath = request.getUri().substring(0, pathEndPos); RestUtils.decodeQueryString(request.getUri(), pathEndPos + 1, params); } } @@ -75,8 +75,8 @@ public class ThriftRestRequest extends AbstractRestRequest implements org.elasti return request.getUri(); } - @Override public String path() { - return this.path; + @Override public String rawPath() { + return this.rawPath; } @Override public boolean hasContent() {