honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-29 03:18:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Stop Passing Around REST Request in Multiple Spots () ()

Browse Source 
* Stop Passing Around REST Request in Multiple Spots

* Motivated by 
  * We are currently passing the REST request object around to a large number of places. This works fine since we simply copy the full request content before we handle the rest itself which is needlessly hard on GC and heap.
  * This PR removes a number of spots where the request is passed around needlessly. There are many more spots to optimize in follow-ups to this, but this one would already enable bypassing the request copying for some error paths in a follow up.
This commit is contained in:
Armin Braun 2019-08-02 07:31:38 +02:00 committed by GitHub
parent e21d58541a
commit 9450505d5b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 104 additions and 186 deletions
modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4
Netty4BadRequestTests.javaNetty4HttpServerTransportTests.java
plugins/transport-nio/src/test/java/org/elasticsearch/http/nio
NioHttpServerTransportTests.java
server/src
main/java/org/elasticsearch
http
AbstractHttpServerTransport.javaHttpServerTransport.java
rest
MethodHandlers.javaRestController.java
test/java/org/elasticsearch
http
AbstractHttpServerTransportTests.java
rest
RestControllerTests.java
test/framework/src/main/java/org/elasticsearch/http
NullDispatcher.java

2
modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4BadRequestTests.java

@ -75,7 +75,7 @@ public class Netty4BadRequestTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadContext threadContext, Throwable cause) { public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
try { try {
final Exception e = cause instanceof Exception ? (Exception) cause : new ElasticsearchException(cause); final Exception e = cause instanceof Exception ? (Exception) cause : new ElasticsearchException(cause);
channel.sendResponse(new BytesRestResponse(channel, RestStatus.BAD_REQUEST, e)); channel.sendResponse(new BytesRestResponse(channel, RestStatus.BAD_REQUEST, e));

13
modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpServerTransportTests.java

@ -152,7 +152,7 @@ public class Netty4HttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadContext threadContext, Throwable cause) { public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
throw new AssertionError(); throw new AssertionError();
} }
}; };
@ -212,10 +212,7 @@ public class Netty4HttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
final RestChannel channel,
final ThreadContext threadContext,
final Throwable cause) {
causeReference.set(cause); causeReference.set(cause);
try { try {
final ElasticsearchException e = new ElasticsearchException("you sent a bad request and you should feel bad"); final ElasticsearchException e = new ElasticsearchException("you sent a bad request and you should feel bad");
@ -272,8 +269,7 @@ public class Netty4HttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel,
final RestChannel channel,
final ThreadContext threadContext, final ThreadContext threadContext,
final Throwable cause) { final Throwable cause) {
throw new AssertionError(); throw new AssertionError();
@ -331,8 +327,7 @@ public class Netty4HttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel,
final RestChannel channel,
final ThreadContext threadContext, final ThreadContext threadContext,
final Throwable cause) { final Throwable cause) {
throw new AssertionError("Should not have received a dispatched request"); throw new AssertionError("Should not have received a dispatched request");

13
plugins/transport-nio/src/test/java/org/elasticsearch/http/nio/NioHttpServerTransportTests.java

@ -149,7 +149,7 @@ public class NioHttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadContext threadContext, Throwable cause) { public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
throw new AssertionError(); throw new AssertionError();
} }
}; };
@ -208,8 +208,7 @@ public class NioHttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel,
final RestChannel channel,
final ThreadContext threadContext, final ThreadContext threadContext,
final Throwable cause) { final Throwable cause) {
throw new AssertionError(); throw new AssertionError();
@ -268,10 +267,7 @@ public class NioHttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
final RestChannel channel,
final ThreadContext threadContext,
final Throwable cause) {
causeReference.set(cause); causeReference.set(cause);
try { try {
final ElasticsearchException e = new ElasticsearchException("you sent a bad request and you should feel bad"); final ElasticsearchException e = new ElasticsearchException("you sent a bad request and you should feel bad");
@ -328,8 +324,7 @@ public class NioHttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel,
final RestChannel channel,
final ThreadContext threadContext, final ThreadContext threadContext,
final Throwable cause) { final Throwable cause) {
throw new AssertionError("Should not have received a dispatched request"); throw new AssertionError("Should not have received a dispatched request");

2
server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java

@ -317,7 +317,7 @@ public abstract class AbstractHttpServerTransport extends AbstractLifecycleCompo
final ThreadContext threadContext = threadPool.getThreadContext(); final ThreadContext threadContext = threadPool.getThreadContext();
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
if (badRequestCause != null) { if (badRequestCause != null) {
dispatcher.dispatchBadRequest(restRequest, channel, threadContext, badRequestCause); dispatcher.dispatchBadRequest(channel, threadContext, badRequestCause);
} else { } else {
dispatcher.dispatchRequest(restRequest, channel, threadContext); dispatcher.dispatchRequest(restRequest, channel, threadContext);
} }

3
server/src/main/java/org/elasticsearch/http/HttpServerTransport.java

@ -54,12 +54,11 @@ public interface HttpServerTransport extends LifecycleComponent {
* Dispatches a bad request. For example, if a request is malformed it will be dispatched via this method with the cause of the bad * Dispatches a bad request. For example, if a request is malformed it will be dispatched via this method with the cause of the bad
* request. * request.
* *
* @param request the request to dispatch
* @param channel the response channel of this request * @param channel the response channel of this request
* @param threadContext the thread context * @param threadContext the thread context
* @param cause the cause of the bad request * @param cause the cause of the bad request
*/ */
void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadContext threadContext, Throwable cause); void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause);
} }
} }

32
server/src/main/java/org/elasticsearch/rest/MethodHandlers.java

@ -19,9 +19,10 @@
package org.elasticsearch.rest; package org.elasticsearch.rest;
import org.elasticsearch.common.Nullable;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.Set; import java.util.Set;
/** /**
@ -40,41 +41,32 @@ final class MethodHandlers {
} }
} }
/**
* Add an additional method and handler for an existing path. Note that {@code MethodHandlers}
* does not allow replacing the handler for an already existing method.
*/
public MethodHandlers addMethod(RestRequest.Method method, RestHandler handler) {
RestHandler existing = methodHandlers.putIfAbsent(method, handler);
if (existing != null) {
throw new IllegalArgumentException("Cannot replace existing handler for [" + path + "] for method: " + method);
}
return this;
}
/** /**
* Add a handler for an additional array of methods. Note that {@code MethodHandlers} * Add a handler for an additional array of methods. Note that {@code MethodHandlers}
* does not allow replacing the handler for an already existing method. * does not allow replacing the handler for an already existing method.
*/ */
public MethodHandlers addMethods(RestHandler handler, RestRequest.Method... methods) { MethodHandlers addMethods(RestHandler handler, RestRequest.Method... methods) {
for (RestRequest.Method method : methods) { for (RestRequest.Method method : methods) {
addMethod(method, handler); RestHandler existing = methodHandlers.putIfAbsent(method, handler);
if (existing != null) {
throw new IllegalArgumentException("Cannot replace existing handler for [" + path + "] for method: " + method);
}
} }
return this; return this;
} }
/** /**
* Return an Optional-wrapped handler for a method, or an empty optional if * Returns the handler for the given method or {@code null} if none exists.
* there is no handler.
*/ */
public Optional<RestHandler> getHandler(RestRequest.Method method) { @Nullable
return Optional.ofNullable(methodHandlers.get(method)); RestHandler getHandler(RestRequest.Method method) {
return methodHandlers.get(method);
} }
/** /**
* Return a set of all valid HTTP methods for the particular path * Return a set of all valid HTTP methods for the particular path
*/ */
public Set<RestRequest.Method> getValidMethods() { Set<RestRequest.Method> getValidMethods() {
return methodHandlers.keySet(); return methodHandlers.keySet();
} }
} }

192
server/src/main/java/org/elasticsearch/rest/RestController.java

@ -75,7 +75,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
/** Rest headers that are copied to internal requests made during a rest request. */ /** Rest headers that are copied to internal requests made during a rest request. */
private final Set<String> headersToCopy; private final Set<String> headersToCopy;
private UsageService usageService; private final UsageService usageService;
public RestController(Set<String> headersToCopy, UnaryOperator<RestHandler> handlerWrapper, public RestController(Set<String> headersToCopy, UnaryOperator<RestHandler> handlerWrapper,
NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) { NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) {
@ -158,17 +158,10 @@ public class RestController implements HttpServerTransport.Dispatcher {
}); });
} }
/**
* @return true iff the circuit breaker limit must be enforced for processing this request.
*/
public boolean canTripCircuitBreaker(final Optional<RestHandler> handler) {
return handler.map(h -> h.canTripCircuitBreaker()).orElse(true);
}
@Override @Override
public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) { public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) {
if (request.rawPath().equals("/favicon.ico")) { if (request.rawPath().equals("/favicon.ico")) {
handleFavicon(request, channel); handleFavicon(request.method(), request.uri(), channel);
return; return;
} }
try { try {
@ -185,8 +178,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, final RestChannel channel, public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
final ThreadContext threadContext, final Throwable cause) {
try { try {
final Exception e; final Exception e;
if (cause == null) { if (cause == null) {
@ -210,62 +202,54 @@ public class RestController implements HttpServerTransport.Dispatcher {
* Dispatch the request, if possible, returning true if a response was sent or false otherwise. * Dispatch the request, if possible, returning true if a response was sent or false otherwise.
*/ */
boolean dispatchRequest(final RestRequest request, final RestChannel channel, final NodeClient client, boolean dispatchRequest(final RestRequest request, final RestChannel channel, final NodeClient client,
final Optional<RestHandler> mHandler) throws Exception { @Nullable final RestHandler handler) throws Exception {
final int contentLength = request.hasContent() ? request.content().length() : 0; if (handler == null) {
// Get the map of matching handlers for a request, for the full set of HTTP methods.
RestChannel responseChannel = channel; final Set<RestRequest.Method> validMethodSet = getValidHandlerMethodSet(request);
// Indicator of whether a response was sent or not final RestRequest.Method method = request.method();
boolean requestHandled; if (validMethodSet.contains(method) == false) {
if (method == RestRequest.Method.OPTIONS) {
if (contentLength > 0 && mHandler.map(h -> hasContentType(request, h) == false).orElse(false)) { handleOptionsRequest(channel, validMethodSet);
sendContentTypeErrorMessage(request, channel); return true;
requestHandled = true; }
} else if (contentLength > 0 && mHandler.map(h -> h.supportsContentStream()).orElse(false) && if (validMethodSet.isEmpty() == false) {
request.getXContentType() != XContentType.JSON && request.getXContentType() != XContentType.SMILE) { // If an alternative handler for an explicit path is registered to a
channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, // different HTTP method than the one supplied - return a 405 Method
RestStatus.NOT_ACCEPTABLE, "Content-Type [" + request.getXContentType() + // Not Allowed error.
"] does not support stream parsing. Use JSON or SMILE instead")); handleUnsupportedHttpMethod(request.uri(), method, channel, validMethodSet, null);
requestHandled = true; return true;
} else if (mHandler.isPresent()) { }
}
return false;
} else {
final int contentLength = request.content().length();
if (contentLength > 0) {
if (hasContentType(request, handler) == false) {
sendContentTypeErrorMessage(request.getAllHeaderValues("Content-Type"), channel);
return true;
}
final XContentType xContentType = request.getXContentType();
if (handler.supportsContentStream() && xContentType != XContentType.JSON && xContentType != XContentType.SMILE) {
channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, RestStatus.NOT_ACCEPTABLE,
"Content-Type [" + xContentType + "] does not support stream parsing. Use JSON or SMILE instead"));
return true;
}
}
RestChannel responseChannel = channel;
try { try {
if (canTripCircuitBreaker(mHandler)) { if (handler.canTripCircuitBreaker()) {
inFlightRequestsBreaker(circuitBreakerService).addEstimateBytesAndMaybeBreak(contentLength, "<http_request>"); inFlightRequestsBreaker(circuitBreakerService).addEstimateBytesAndMaybeBreak(contentLength, "<http_request>");
} else { } else {
inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(contentLength); inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(contentLength);
} }
// iff we could reserve bytes for the request we need to send the response also over this channel // iff we could reserve bytes for the request we need to send the response also over this channel
responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength); responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength);
handler.handleRequest(request, responseChannel, client);
final RestHandler wrappedHandler = mHandler.get();
wrappedHandler.handleRequest(request, responseChannel, client);
requestHandled = true;
} catch (Exception e) { } catch (Exception e) {
responseChannel.sendResponse(new BytesRestResponse(responseChannel, e)); responseChannel.sendResponse(new BytesRestResponse(responseChannel, e));
// We "handled" the request by returning a response, even though it was an error
requestHandled = true;
}
} else {
// Get the map of matching handlers for a request, for the full set of HTTP methods.
final Set<RestRequest.Method> validMethodSet = getValidHandlerMethodSet(request);
if (validMethodSet.size() > 0
&& validMethodSet.contains(request.method()) == false
&& request.method() != RestRequest.Method.OPTIONS) {
// If an alternative handler for an explicit path is registered to a
// different HTTP method than the one supplied - return a 405 Method
// Not Allowed error.
handleUnsupportedHttpMethod(request, channel, validMethodSet, null);
requestHandled = true;
} else if (validMethodSet.contains(request.method()) == false
&& (request.method() == RestRequest.Method.OPTIONS)) {
handleOptionsRequest(request, channel, validMethodSet);
requestHandled = true;
} else {
requestHandled = false;
} }
return true;
} }
// Return true if the request was handled, false otherwise.
return requestHandled;
} }
/** /**
@ -288,14 +272,13 @@ public class RestController implements HttpServerTransport.Dispatcher {
return true; return true;
} }
private void sendContentTypeErrorMessage(RestRequest restRequest, RestChannel channel) throws IOException { private void sendContentTypeErrorMessage(@Nullable List<String> contentTypeHeader, RestChannel channel) throws IOException {
final List<String> contentTypeHeader = restRequest.getAllHeaderValues("Content-Type");
final String errorMessage; final String errorMessage;
if (contentTypeHeader == null) { if (contentTypeHeader == null) {
errorMessage = "Content-Type header is missing"; errorMessage = "Content-Type header is missing";
} else { } else {
errorMessage = "Content-Type header [" + errorMessage = "Content-Type header [" +
Strings.collectionToCommaDelimitedString(restRequest.getAllHeaderValues("Content-Type")) + "] is not supported"; Strings.collectionToCommaDelimitedString(contentTypeHeader) + "] is not supported";
} }
channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, NOT_ACCEPTABLE, errorMessage)); channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, NOT_ACCEPTABLE, errorMessage));
@ -305,26 +288,19 @@ public class RestController implements HttpServerTransport.Dispatcher {
* Checks the request parameters against enabled settings for error trace support * Checks the request parameters against enabled settings for error trace support
* @return true if the request does not have any parameters that conflict with system settings * @return true if the request does not have any parameters that conflict with system settings
*/ */
boolean checkErrorTraceParameter(final RestRequest request, final RestChannel channel) { private static boolean checkErrorTraceParameter(final RestRequest request, final RestChannel channel) {
// error_trace cannot be used when we disable detailed errors // error_trace cannot be used when we disable detailed errors
// we consume the error_trace parameter first to ensure that it is always consumed // we consume the error_trace parameter first to ensure that it is always consumed
if (request.paramAsBoolean("error_trace", false) && channel.detailedErrorsEnabled() == false) { return request.paramAsBoolean("error_trace", false) == false || channel.detailedErrorsEnabled();
return false;
}
return true;
} }
void tryAllHandlers(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) throws Exception { private void tryAllHandlers(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) throws Exception {
for (String key : headersToCopy) { for (String key : headersToCopy) {
String httpHeader = request.header(key); String httpHeader = request.header(key);
if (httpHeader != null) { if (httpHeader != null) {
threadContext.putHeader(key, httpHeader); threadContext.putHeader(key, httpHeader);
} }
} }
// Request execution flag
boolean requestHandled = false;
if (checkErrorTraceParameter(request, channel) == false) { if (checkErrorTraceParameter(request, channel) == false) {
channel.sendResponse( channel.sendResponse(
BytesRestResponse.createSimpleErrorResponse(channel, BAD_REQUEST, "error traces in responses are disabled.")); BytesRestResponse.createSimpleErrorResponse(channel, BAD_REQUEST, "error traces in responses are disabled."));
@ -334,37 +310,37 @@ public class RestController implements HttpServerTransport.Dispatcher {
try { try {
// Resolves the HTTP method and fails if the method is invalid // Resolves the HTTP method and fails if the method is invalid
final RestRequest.Method requestMethod = request.method(); final RestRequest.Method requestMethod = request.method();
// Loop through all possible handlers, attempting to dispatch the request // Loop through all possible handlers, attempting to dispatch the request
Iterator<MethodHandlers> allHandlers = getAllHandlers(request); Iterator<MethodHandlers> allHandlers = getAllHandlers(request);
for (Iterator<MethodHandlers> it = allHandlers; it.hasNext(); ) { while (allHandlers.hasNext()) {
Optional<RestHandler> mHandler = Optional.empty(); final RestHandler handler;
if (requestMethod != null) { final MethodHandlers handlers = allHandlers.next();
mHandler = Optional.ofNullable(it.next()).flatMap(mh -> mh.getHandler(requestMethod)); if (handlers == null) {
handler = null;
} else {
handler = handlers.getHandler(requestMethod);
} }
requestHandled = dispatchRequest(request, channel, client, mHandler); if (dispatchRequest(request, channel, client, handler)) {
if (requestHandled) { return;
break;
} }
} }
} catch (final IllegalArgumentException e) { } catch (final IllegalArgumentException e) {
handleUnsupportedHttpMethod(request, channel, getValidHandlerMethodSet(request), e); handleUnsupportedHttpMethod(request.uri(), null, channel, getValidHandlerMethodSet(request), e);
requestHandled = true; return;
} }
// If request has not been handled, fallback to a bad request error. // If request has not been handled, fallback to a bad request error.
if (requestHandled == false) { handleBadRequest(request.uri(), request.method(), channel);
handleBadRequest(request, channel);
}
} }
Iterator<MethodHandlers> getAllHandlers(final RestRequest request) { Iterator<MethodHandlers> getAllHandlers(final RestRequest request) {
// Between retrieving the correct path, we need to reset the parameters, // Between retrieving the correct path, we need to reset the parameters,
// otherwise parameters are parsed out of the URI that aren't actually handled. // otherwise parameters are parsed out of the URI that aren't actually handled.
final Map<String, String> originalParams = new HashMap<>(request.params()); final Map<String, String> originalParams = new HashMap<>(request.params());
// 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
final Map<String, String> requestParamsRef = request.params(); final Map<String, String> requestParamsRef = request.params();
return handlers.retrieveAll(getPath(request), () -> { return handlers.retrieveAll(request.rawPath(), () -> {
// PathTrie modifies the request, so reset the params between each iteration // PathTrie modifies the request, so reset the params between each iteration
requestParamsRef.clear(); requestParamsRef.clear();
requestParamsRef.putAll(originalParams); requestParamsRef.putAll(originalParams);
@ -379,17 +355,18 @@ public class RestController implements HttpServerTransport.Dispatcher {
* <a href="https://tools.ietf.org/html/rfc2616#section-10.4.6">HTTP/1.1 - * <a href="https://tools.ietf.org/html/rfc2616#section-10.4.6">HTTP/1.1 -
* 10.4.6 - 405 Method Not Allowed</a>). * 10.4.6 - 405 Method Not Allowed</a>).
*/ */
private void handleUnsupportedHttpMethod(final RestRequest request, private void handleUnsupportedHttpMethod(String uri,
@Nullable RestRequest.Method method,
final RestChannel channel, final RestChannel channel,
final Set<RestRequest.Method> validMethodSet, final Set<RestRequest.Method> validMethodSet,
@Nullable final IllegalArgumentException exception) { @Nullable final IllegalArgumentException exception) {
try { try {
final StringBuilder msg = new StringBuilder(); final StringBuilder msg = new StringBuilder();
if (exception != null) { if (exception == null) {
msg.append(exception.getMessage()); msg.append("Incorrect HTTP method for uri [").append(uri);
msg.append("] and method [").append(method).append("]");
} else { } else {
msg.append("Incorrect HTTP method for uri [").append(request.uri()); msg.append(exception.getMessage());
msg.append("] and method [").append(request.method()).append("]");
} }
if (validMethodSet.isEmpty() == false) { if (validMethodSet.isEmpty() == false) {
msg.append(", allowed: ").append(validMethodSet); msg.append(", allowed: ").append(validMethodSet);
@ -412,31 +389,25 @@ public class RestController implements HttpServerTransport.Dispatcher {
* <a href="https://tools.ietf.org/html/rfc2616#section-9.2">HTTP/1.1 - 9.2 * <a href="https://tools.ietf.org/html/rfc2616#section-9.2">HTTP/1.1 - 9.2
* - Options</a>). * - Options</a>).
*/ */
private void handleOptionsRequest(RestRequest request, RestChannel channel, Set<RestRequest.Method> validMethodSet) { private void handleOptionsRequest(RestChannel channel, Set<RestRequest.Method> validMethodSet) {
assert request.method() == RestRequest.Method.OPTIONS; BytesRestResponse bytesRestResponse = new BytesRestResponse(OK, TEXT_CONTENT_TYPE, BytesArray.EMPTY);
// When we have an OPTIONS HTTP request and no valid handlers, simply send OK by default (with the Access Control Origin header
// which gets automatically added).
if (validMethodSet.isEmpty() == false) { if (validMethodSet.isEmpty() == false) {
BytesRestResponse bytesRestResponse = new BytesRestResponse(OK, TEXT_CONTENT_TYPE, BytesArray.EMPTY);
bytesRestResponse.addHeader("Allow", Strings.collectionToDelimitedString(validMethodSet, ",")); bytesRestResponse.addHeader("Allow", Strings.collectionToDelimitedString(validMethodSet, ","));
channel.sendResponse(bytesRestResponse);
} else {
/*
* When we have an OPTIONS HTTP request and no valid handlers,
* simply send OK by default (with the Access Control Origin header
* which gets automatically added).
*/
channel.sendResponse(new BytesRestResponse(OK, TEXT_CONTENT_TYPE, BytesArray.EMPTY));
} }
channel.sendResponse(bytesRestResponse);
} }
/** /**
* Handle a requests with no candidate handlers (return a 400 Bad Request * Handle a requests with no candidate handlers (return a 400 Bad Request
* error). * error).
*/ */
private void handleBadRequest(RestRequest request, RestChannel channel) throws IOException { private void handleBadRequest(String uri, RestRequest.Method method, RestChannel channel) throws IOException {
try (XContentBuilder builder = channel.newErrorBuilder()) { try (XContentBuilder builder = channel.newErrorBuilder()) {
builder.startObject(); builder.startObject();
{ {
builder.field("error", "no handler found for uri [" + request.uri() + "] and method [" + request.method() + "]"); builder.field("error", "no handler found for uri [" + uri + "] and method [" + method + "]");
} }
builder.endObject(); builder.endObject();
channel.sendResponse(new BytesRestResponse(BAD_REQUEST, builder)); channel.sendResponse(new BytesRestResponse(BAD_REQUEST, builder));
@ -455,17 +426,10 @@ public class RestController implements HttpServerTransport.Dispatcher {
return validMethods; return validMethods;
} }
private String getPath(RestRequest request) { private void handleFavicon(RestRequest.Method method, String uri, final RestChannel channel) {
// 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();
}
private void handleFavicon(final RestRequest request, final RestChannel channel) {
try { try {
if (request.method() != RestRequest.Method.GET) { if (method != RestRequest.Method.GET) {
handleUnsupportedHttpMethod(request, channel, Collections.singleton(RestRequest.Method.GET), null); handleUnsupportedHttpMethod(uri, method, channel, Collections.singleton(RestRequest.Method.GET), null);
} else { } else {
try { try {
try (InputStream stream = getClass().getResourceAsStream("/config/favicon.ico")) { try (InputStream stream = getClass().getResourceAsStream("/config/favicon.ico")) {
@ -480,7 +444,7 @@ public class RestController implements HttpServerTransport.Dispatcher {
} }
} }
} catch (final IllegalArgumentException e) { } catch (final IllegalArgumentException e) {
handleUnsupportedHttpMethod(request, channel, Collections.singleton(RestRequest.Method.GET), e); handleUnsupportedHttpMethod(uri, null, channel, Collections.singleton(RestRequest.Method.GET), e);
} }
} }

3
server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java

@ -115,8 +115,7 @@ public class AbstractHttpServerTransportTests extends ESTestCase {
} }
@Override @Override
public void dispatchBadRequest(final RestRequest request, public void dispatchBadRequest(final RestChannel channel,
final RestChannel channel,
final ThreadContext threadContext, final ThreadContext threadContext,
final Throwable cause) { final Throwable cause) {
threadContext.putHeader("foo_bad", "bar"); threadContext.putHeader("foo_bad", "bar");

28
server/src/test/java/org/elasticsearch/rest/RestControllerTests.java

@ -53,7 +53,6 @@ import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
@ -137,30 +136,6 @@ public class RestControllerTests extends ESTestCase {
assertNull(threadContext.getHeader("header.3")); assertNull(threadContext.getHeader("header.3"));
} }
public void testCanTripCircuitBreaker() throws Exception {
RestController controller = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService);
// trip circuit breaker by default
controller.registerHandler(RestRequest.Method.GET, "/trip", new FakeRestHandler(true));
controller.registerHandler(RestRequest.Method.GET, "/do-not-trip", new FakeRestHandler(false));
RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withPath("/trip").build();
for (Iterator<MethodHandlers> it = controller.getAllHandlers(fakeRequest); it.hasNext(); ) {
Optional<MethodHandlers> mHandler = Optional.ofNullable(it.next());
assertTrue(mHandler.map(mh -> controller.canTripCircuitBreaker(mh.getHandler(RestRequest.Method.GET))).orElse(true));
}
// assume trip even on unknown paths
fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withPath("/unknown-path").build();
for (Iterator<MethodHandlers> it = controller.getAllHandlers(fakeRequest); it.hasNext(); ) {
Optional<MethodHandlers> mHandler = Optional.ofNullable(it.next());
assertTrue(mHandler.map(mh -> controller.canTripCircuitBreaker(mh.getHandler(RestRequest.Method.GET))).orElse(true));
}
fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withPath("/do-not-trip").build();
for (Iterator<MethodHandlers> it = controller.getAllHandlers(fakeRequest); it.hasNext(); ) {
Optional<MethodHandlers> mHandler = Optional.ofNullable(it.next());
assertFalse(mHandler.map(mh -> controller.canTripCircuitBreaker(mh.getHandler(RestRequest.Method.GET))).orElse(false));
}
}
public void testRegisterAsDeprecatedHandler() { public void testRegisterAsDeprecatedHandler() {
RestController controller = mock(RestController.class); RestController controller = mock(RestController.class);
@ -459,7 +434,6 @@ public class RestControllerTests extends ESTestCase {
final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build();
final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST); final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST);
restController.dispatchBadRequest( restController.dispatchBadRequest(
fakeRestRequest,
channel, channel,
new ThreadContext(Settings.EMPTY), new ThreadContext(Settings.EMPTY),
randomBoolean() ? new IllegalStateException("bad request") : new Throwable("bad request")); randomBoolean() ? new IllegalStateException("bad request") : new Throwable("bad request"));
@ -470,7 +444,7 @@ public class RestControllerTests extends ESTestCase {
public void testDispatchBadRequestUnknownCause() { public void testDispatchBadRequestUnknownCause() {
final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build();
final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST); final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST);
restController.dispatchBadRequest(fakeRestRequest, channel, new ThreadContext(Settings.EMPTY), null); restController.dispatchBadRequest(channel, new ThreadContext(Settings.EMPTY), null);
assertTrue(channel.getSendResponseCalled()); assertTrue(channel.getSendResponseCalled());
assertThat(channel.getRestResponse().content().utf8ToString(), containsString("unknown cause")); assertThat(channel.getRestResponse().content().utf8ToString(), containsString("unknown cause"));
} }

2
test/framework/src/main/java/org/elasticsearch/http/NullDispatcher.java

@ -31,7 +31,7 @@ public class NullDispatcher implements HttpServerTransport.Dispatcher {
} }
@Override @Override
public void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadContext threadContext, Throwable cause) { public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) {
} }