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 <[email protected]> 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"); } }; }
