You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2018/08/27 14:08:50 UTC
[camel] 03/03: CAMEL-12753: Fixed camel-netty4-http consumer with
OPTIONS in rest-dsl to return mutiple verbs if the same context-path has
more http verbs like GET, POST etc. Thanks to Tomas Turek for unit test
This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/camel.git
commit e3b5a4ee7719965f1b42b27c6a4d7a7b9222970f
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Mon Aug 27 16:08:22 2018 +0200
CAMEL-12753: Fixed camel-netty4-http consumer with OPTIONS in rest-dsl to return mutiple verbs if the same context-path has more http verbs like GET,POST etc. Thanks to Tomas Turek for unit test
---
.../common/HttpServletResolveConsumerStrategy.java | 1 -
.../http/handlers/HttpServerChannelHandler.java | 22 ++------
.../HttpServerMultiplexChannelHandler.java | 62 ++++++++++++++++++----
.../netty4/http/rest/RestNettyHttpOptionsTest.java | 28 ++++++++--
4 files changed, 78 insertions(+), 35 deletions(-)
diff --git a/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java b/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java
index 9b369c3..b31daa5 100644
--- a/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java
+++ b/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpServletResolveConsumerStrategy.java
@@ -78,5 +78,4 @@ public class HttpServletResolveConsumerStrategy implements ServletResolveConsume
return restrict == null || restrict.toLowerCase(Locale.ENGLISH).contains(method.toLowerCase(Locale.ENGLISH));
}
-
}
diff --git a/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerChannelHandler.java b/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerChannelHandler.java
index 3034a56..2909f09 100644
--- a/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerChannelHandler.java
+++ b/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerChannelHandler.java
@@ -19,8 +19,11 @@ package org.apache.camel.component.netty4.http.handlers;
import java.net.URI;
import java.nio.channels.ClosedChannelException;
import java.nio.charset.Charset;
+import java.util.Arrays;
import java.util.Iterator;
+import java.util.List;
import java.util.Locale;
+import java.util.stream.Collectors;
import javax.security.auth.Subject;
import javax.security.auth.login.LoginException;
@@ -43,6 +46,7 @@ import org.apache.camel.component.netty4.http.HttpPrincipal;
import org.apache.camel.component.netty4.http.NettyHttpConsumer;
import org.apache.camel.component.netty4.http.NettyHttpSecurityConfiguration;
import org.apache.camel.component.netty4.http.SecurityAuthenticator;
+import org.apache.camel.http.common.CamelServlet;
import org.apache.camel.util.CamelLogger;
import org.apache.camel.util.ObjectHelper;
import org.slf4j.Logger;
@@ -91,24 +95,6 @@ public class HttpServerChannelHandler extends ServerChannelHandler {
return;
}
- // if its an OPTIONS request then return which methods is allowed
- boolean isRestrictedToOptions = consumer.getEndpoint().getHttpMethodRestrict() != null
- && consumer.getEndpoint().getHttpMethodRestrict().contains("OPTIONS");
- if ("OPTIONS".equals(request.method().name()) && !isRestrictedToOptions) {
- String s;
- if (consumer.getEndpoint().getHttpMethodRestrict() != null) {
- s = "OPTIONS," + consumer.getEndpoint().getHttpMethodRestrict();
- } else {
- // allow them all
- s = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH";
- }
- HttpResponse response = new DefaultHttpResponse(HTTP_1_1, OK);
- response.headers().set("Allow", s);
- // do not include content-type as that would indicate to the caller that we can only do text/plain
- response.headers().set(Exchange.CONTENT_LENGTH, 0);
- ctx.writeAndFlush(response);
- return;
- }
if (consumer.getEndpoint().getHttpMethodRestrict() != null
&& !consumer.getEndpoint().getHttpMethodRestrict().contains(request.method().name())) {
HttpResponse response = new DefaultHttpResponse(HTTP_1_1, METHOD_NOT_ALLOWED);
diff --git a/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerMultiplexChannelHandler.java b/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerMultiplexChannelHandler.java
index 27fb206..0fddb8b 100644
--- a/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerMultiplexChannelHandler.java
+++ b/components/camel-netty4-http/src/main/java/org/apache/camel/component/netty4/http/handlers/HttpServerMultiplexChannelHandler.java
@@ -19,8 +19,10 @@ package org.apache.camel.component.netty4.http.handlers;
import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
import java.util.List;
+import java.util.Locale;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.stream.Collectors;
import io.netty.channel.ChannelHandler;
@@ -44,6 +46,7 @@ import org.slf4j.LoggerFactory;
import static io.netty.handler.codec.http.HttpResponseStatus.METHOD_NOT_ALLOWED;
import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
+import static io.netty.handler.codec.http.HttpResponseStatus.OK;
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
/**
@@ -105,15 +108,41 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
HttpServerChannelHandler handler = getHandler(request, request.method().name());
if (handler != null) {
- Attribute<HttpServerChannelHandler> attr = ctx.channel().attr(SERVER_HANDLER_KEY);
- // store handler as attachment
- attr.set(handler);
- if (msg instanceof HttpContent) {
- // need to hold the reference of content
- HttpContent httpContent = (HttpContent) msg;
- httpContent.content().retain();
- }
- handler.channelRead(ctx, request);
+
+ // special if its an OPTIONS request
+ boolean isRestrictedToOptions = handler.getConsumer().getEndpoint().getHttpMethodRestrict() != null
+ && handler.getConsumer().getEndpoint().getHttpMethodRestrict().contains("OPTIONS");
+ if ("OPTIONS".equals(request.method().name()) && !isRestrictedToOptions) {
+ String allowedMethods = CamelServlet.METHODS.stream().filter((m) -> isHttpMethodAllowed(request, m)).collect(Collectors.joining(","));
+ if (allowedMethods == null && handler.getConsumer().getEndpoint().getHttpMethodRestrict() != null) {
+ allowedMethods = handler.getConsumer().getEndpoint().getHttpMethodRestrict();
+ }
+
+ if (allowedMethods == null) {
+ allowedMethods = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH";
+ }
+
+ if (!allowedMethods.contains("OPTIONS")) {
+ allowedMethods = allowedMethods + ",OPTIONS";
+ }
+
+ HttpResponse response = new DefaultHttpResponse(HTTP_1_1, OK);
+ response.headers().set(Exchange.CONTENT_TYPE, "text/plain");
+ response.headers().set(Exchange.CONTENT_LENGTH, 0);
+ response.headers().set("Allow", allowedMethods);
+ ctx.writeAndFlush(response);
+ ctx.close();
+ } else {
+ Attribute<HttpServerChannelHandler> attr = ctx.channel().attr(SERVER_HANDLER_KEY);
+ // store handler as attachment
+ attr.set(handler);
+ if (msg instanceof HttpContent) {
+ // need to hold the reference of content
+ HttpContent httpContent = (HttpContent) msg;
+ httpContent.content().retain();
+ }
+ handler.channelRead(ctx, request);
+ }
} else {
// okay we cannot process this requires so return either 404 or 405.
// to know if its 405 then we need to check if any other HTTP method would have a consumer for the "same" request
@@ -188,7 +217,9 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
answer = best.getConsumer();
}
+
// fallback to regular matching
+ List<HttpServerChannelHandler> candidates = new ArrayList<>();
if (answer == null) {
for (final HttpServerChannelHandler handler : consumers) {
NettyHttpConsumer consumer = handler.getConsumer();
@@ -196,12 +227,17 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
boolean matchOnUriPrefix = consumer.getEndpoint().getConfiguration().isMatchOnUriPrefix();
// Just make sure the we get the right consumer path first
if (RestConsumerContextPathMatcher.matchPath(path, consumerPath, matchOnUriPrefix)) {
- answer = handler;
- break;
+ candidates.add(handler);
}
}
}
+ // extra filter by restrict
+ candidates = candidates.stream().filter(c -> matchRestMethod(method, c.getConsumer().getEndpoint().getHttpMethodRestrict())).collect(Collectors.toList());
+ if (candidates.size() == 1) {
+ answer = candidates.get(0);
+ }
+
return answer;
}
@@ -225,4 +261,8 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
return UnsafeUriCharactersEncoder.encodeHttpURI(path);
}
+ private static boolean matchRestMethod(String method, String restrict) {
+ return restrict == null || restrict.toLowerCase(Locale.ENGLISH).contains(method.toLowerCase(Locale.ENGLISH));
+ }
+
}
diff --git a/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyHttpOptionsTest.java b/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyHttpOptionsTest.java
index a6ea494..717bb7a 100644
--- a/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyHttpOptionsTest.java
+++ b/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyHttpOptionsTest.java
@@ -34,12 +34,26 @@ public class RestNettyHttpOptionsTest extends BaseNettyTest {
});
assertEquals(200, exchange.getOut().getHeader(Exchange.HTTP_RESPONSE_CODE));
- assertEquals("OPTIONS,GET", exchange.getOut().getHeader("ALLOW"));
+ assertEquals("GET,OPTIONS", exchange.getOut().getHeader("ALLOW"));
assertEquals("", exchange.getOut().getBody(String.class));
- exchange = fluentTemplate.to("http://localhost:" + getPort() + "/users/v1/123").withHeader(Exchange.HTTP_METHOD, "OPTIONS").send();
+ exchange = fluentTemplate.to("http://localhost:" + getPort() + "/users/v1/id/123").withHeader(Exchange.HTTP_METHOD, "OPTIONS").send();
assertEquals(200, exchange.getOut().getHeader(Exchange.HTTP_RESPONSE_CODE));
- assertEquals("OPTIONS,PUT", exchange.getOut().getHeader("ALLOW"));
+ assertEquals("PUT,OPTIONS", exchange.getOut().getHeader("ALLOW"));
+ assertEquals("", exchange.getOut().getBody(String.class));
+ }
+
+ @Test
+ public void testNettyServerMultipleOptions() throws Exception {
+ Exchange exchange = template.request("http://localhost:" + getPort() + "/users/v2/options", new Processor() {
+ @Override
+ public void process(Exchange exchange) throws Exception {
+ exchange.getIn().setHeader(Exchange.HTTP_METHOD, "OPTIONS");
+ }
+ });
+
+ assertEquals(200, exchange.getOut().getHeader(Exchange.HTTP_RESPONSE_CODE));
+ assertEquals("GET,POST,OPTIONS", exchange.getOut().getHeader("ALLOW"));
assertEquals("", exchange.getOut().getBody(String.class));
}
@@ -55,8 +69,12 @@ public class RestNettyHttpOptionsTest extends BaseNettyTest {
rest("/users/")
.get("v1/customers")
.to("mock:customers")
- .put("v1/{id}")
- .to("mock:id");
+ .put("v1/id/{id}")
+ .to("mock:id")
+ .get("v2/options")
+ .to("mock:options")
+ .post("v2/options")
+ .to("mock:options");
}
};
}