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:47 UTC

[camel] branch master updated (a0c2e10 -> e3b5a4e)

This is an automated email from the ASF dual-hosted git repository.

davsclaus pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/camel.git.


    from a0c2e10  CAMEL-12752: Upgrade Jetty to 9.4.11
     new c3cd895  CAMEL-12753: Fixed camel-undertow 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
     new 7331d83  CAMEL-12753: Fixed camel-servlet,camel-jetty 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
     new e3b5a4e  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

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/camel/http/common/CamelServlet.java | 20 ++++---
 .../common/HttpServletResolveConsumerStrategy.java |  9 +---
 .../common/ServletResolveConsumerStrategy.java     |  4 +-
 .../component/jetty/CamelContinuationServlet.java  | 17 +++---
 .../component/jetty/rest/RestJettyOptionsTest.java | 28 ++++++++--
 .../http/handlers/HttpServerChannelHandler.java    | 22 ++------
 .../HttpServerMultiplexChannelHandler.java         | 62 ++++++++++++++++++----
 .../netty4/http/rest/RestNettyHttpOptionsTest.java | 28 ++++++++--
 .../servlet/rest/RestServletOptionsTest.java       | 26 +++++++--
 .../undertow/HttpHandlerRegistrationInfo.java      | 20 +++++++
 .../component/undertow/UndertowComponent.java      | 13 +++--
 .../camel/component/undertow/UndertowConsumer.java | 33 +++++++++---
 .../undertow/rest/RestUndertowHttpOptionsTest.java | 19 ++++++-
 13 files changed, 225 insertions(+), 76 deletions(-)


[camel] 02/03: CAMEL-12753: Fixed camel-servlet, camel-jetty 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

Posted by da...@apache.org.
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 7331d83f073ab52dcc846efb7b5b8582e44b590b
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Mon Aug 27 15:48:34 2018 +0200

    CAMEL-12753: Fixed camel-servlet,camel-jetty 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
---
 .../org/apache/camel/http/common/CamelServlet.java | 20 ++++++++++------
 .../common/HttpServletResolveConsumerStrategy.java |  8 ++-----
 .../common/ServletResolveConsumerStrategy.java     |  4 ++--
 .../component/jetty/CamelContinuationServlet.java  | 17 ++++++++-----
 .../component/jetty/rest/RestJettyOptionsTest.java | 28 ++++++++++++++++++----
 .../servlet/rest/RestServletOptionsTest.java       | 26 ++++++++++++++++----
 6 files changed, 72 insertions(+), 31 deletions(-)

diff --git a/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java b/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java
index 9df103f..166cb2e 100644
--- a/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java
+++ b/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java
@@ -23,6 +23,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.stream.Collector;
+import java.util.stream.Collectors;
 import javax.servlet.AsyncContext;
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
@@ -141,14 +143,18 @@ public class CamelServlet extends HttpServlet {
 
         // if its an OPTIONS request then return which method is allowed
         if ("OPTIONS".equals(request.getMethod()) && !consumer.isOptionsEnabled()) {
-            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";
+            String allowedMethods = METHODS.stream().filter(m -> getServletResolveConsumerStrategy().isHttpMethodAllowed(request, m, getConsumers())).collect(Collectors.joining(","));
+            if (allowedMethods == null && consumer.getEndpoint().getHttpMethodRestrict() != null) {
+                allowedMethods = consumer.getEndpoint().getHttpMethodRestrict();
+            }
+            if (allowedMethods == null) {
+               // allow them all
+                allowedMethods = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH";
+            }
+            if (!allowedMethods.contains("OPTIONS")) {
+                allowedMethods = allowedMethods + ",OPTIONS";
             }
-            response.addHeader("Allow", s);
+            response.addHeader("Allow", allowedMethods);
             response.setStatus(HttpServletResponse.SC_OK);
             return;
         }
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 e08827a..9b369c3 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
@@ -48,14 +48,10 @@ public class HttpServletResolveConsumerStrategy implements ServletResolveConsume
         HttpConsumer answer = consumers.get(path);
 
         List<HttpConsumer> candidates = resolveCandidates(request, method, consumers);
+        // extra filter by restrict
+        candidates = candidates.stream().filter(c -> matchRestMethod(method, c.getEndpoint().getHttpMethodRestrict())).collect(Collectors.toList());
         if (candidates.size() == 1) {
             answer = candidates.get(0);
-        } else {
-            // extra filter by restrict
-            candidates = candidates.stream().filter(c -> matchRestMethod(method, c.getEndpoint().getHttpMethodRestrict())).collect(Collectors.toList());
-            if (candidates.size() == 1) {
-                answer = candidates.get(0);
-            }
         }
 
         return answer;
diff --git a/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java b/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java
index 039f74b..c1ea400 100644
--- a/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java
+++ b/components/camel-http-common/src/main/java/org/apache/camel/http/common/ServletResolveConsumerStrategy.java
@@ -40,8 +40,8 @@ public interface ServletResolveConsumerStrategy {
      * @param request   the http request
      * @param method    the http method
      * @param consumers the map of registered consumers
-     * @return the consumer to service the request, or <tt>null</tt> if no match,
-     * which sends back a {@link javax.servlet.http.HttpServletResponse#SC_METHOD_NOT_ALLOWED} to the client.
+     * @return <tt>true</tt> if the method is allowed and can be serviced. Otherwise a
+     * {@link javax.servlet.http.HttpServletResponse#SC_METHOD_NOT_ALLOWED} is returned to the client.
      */
     boolean isHttpMethodAllowed(HttpServletRequest request, String method, Map<String, HttpConsumer> consumers);
 
diff --git a/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java b/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java
index 5e4cdff..d0a16d5 100644
--- a/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java
+++ b/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/CamelContinuationServlet.java
@@ -21,6 +21,7 @@ import java.util.Iterator;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeoutException;
+import java.util.stream.Collectors;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -105,14 +106,18 @@ public class CamelContinuationServlet extends CamelServlet {
 
         // if its an OPTIONS request then return which method is allowed
         if ("OPTIONS".equals(request.getMethod()) && !consumer.isOptionsEnabled()) {
-            String s;
-            if (consumer.getEndpoint().getHttpMethodRestrict() != null) {
-                s = "OPTIONS," + consumer.getEndpoint().getHttpMethodRestrict();
-            } else {
+            String allowedMethods = METHODS.stream().filter(m -> getServletResolveConsumerStrategy().isHttpMethodAllowed(request, m, getConsumers())).collect(Collectors.joining(","));
+            if (allowedMethods == null && consumer.getEndpoint().getHttpMethodRestrict() != null) {
+                allowedMethods = consumer.getEndpoint().getHttpMethodRestrict();
+            }
+            if (allowedMethods == null) {
                 // allow them all
-                s = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH";
+                allowedMethods = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH";
+            }
+            if (!allowedMethods.contains("OPTIONS")) {
+                allowedMethods = allowedMethods + ",OPTIONS";
             }
-            response.addHeader("Allow", s);
+            response.addHeader("Allow", allowedMethods);
             response.setStatus(HttpServletResponse.SC_OK);
             return;
         }
diff --git a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java
index 7f6536e..34577177 100644
--- a/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java
+++ b/components/camel-jetty9/src/test/java/org/apache/camel/component/jetty/rest/RestJettyOptionsTest.java
@@ -34,12 +34,26 @@ public class RestJettyOptionsTest extends BaseJettyTest {
         });
 
         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 testJettyServerMultipleOptions() 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 RestJettyOptionsTest extends BaseJettyTest {
                 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");
             }
         };
     }
diff --git a/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java
index f6df318..b5454f1 100644
--- a/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java
+++ b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletOptionsTest.java
@@ -42,14 +42,26 @@ public class RestServletOptionsTest extends ServletCamelRouterTestSupport {
         WebResponse response = client.getResponse(req);
 
         assertEquals(200, response.getResponseCode());
-        assertEquals("OPTIONS,GET", response.getHeaderField("ALLOW"));
+        assertEquals("GET,OPTIONS", response.getHeaderField("ALLOW"));
         assertEquals("", response.getText());
 
-        req = new OptionsMethodWebRequest(CONTEXT_URL + "/services/users/v1/123");
+        req = new OptionsMethodWebRequest(CONTEXT_URL + "/services/users/v1/id/123");
         response = client.getResponse(req);
 
         assertEquals(200, response.getResponseCode());
-        assertEquals("OPTIONS,PUT", response.getHeaderField("ALLOW"));
+        assertEquals("PUT,OPTIONS", response.getHeaderField("ALLOW"));
+        assertEquals("", response.getText());
+    }
+
+    @Test
+    public void testMultipleServletOptions() throws Exception {
+        WebRequest req = new OptionsMethodWebRequest(CONTEXT_URL + "/services/users/v2/options");
+        ServletUnitClient client = newClient();
+        client.setExceptionsThrownOnErrorStatus(false);
+        WebResponse response = client.getResponse(req);
+
+        assertEquals(200, response.getResponseCode());
+        assertEquals("GET,POST,OPTIONS", response.getHeaderField("ALLOW"));
         assertEquals("", response.getText());
     }
 
@@ -65,8 +77,12 @@ public class RestServletOptionsTest extends ServletCamelRouterTestSupport {
                 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");
             }
         };
     }


[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

Posted by da...@apache.org.
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");
             }
         };
     }


[camel] 01/03: CAMEL-12753: Fixed camel-undertow 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

Posted by da...@apache.org.
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 c3cd89585c4a437767a8e72aee388f3ed17c62e9
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Mon Aug 27 14:58:57 2018 +0200

    CAMEL-12753: Fixed camel-undertow 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
---
 .../undertow/HttpHandlerRegistrationInfo.java      | 20 +++++++++++++
 .../component/undertow/UndertowComponent.java      | 13 +++++++--
 .../camel/component/undertow/UndertowConsumer.java | 33 ++++++++++++++++++----
 .../undertow/rest/RestUndertowHttpOptionsTest.java | 19 ++++++++++++-
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/HttpHandlerRegistrationInfo.java b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/HttpHandlerRegistrationInfo.java
index ce93093..9e55856 100644
--- a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/HttpHandlerRegistrationInfo.java
+++ b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/HttpHandlerRegistrationInfo.java
@@ -17,6 +17,7 @@
 package org.apache.camel.component.undertow;
 
 import java.net.URI;
+import java.util.Objects;
 
 public class HttpHandlerRegistrationInfo {
 
@@ -48,4 +49,23 @@ public class HttpHandlerRegistrationInfo {
         return uri + "?matchOnUriPrefix=" + matchOnUriPrefix + "&methodRestrict=" + methodRestrict;
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        HttpHandlerRegistrationInfo that = (HttpHandlerRegistrationInfo) o;
+        return Objects.equals(matchOnUriPrefix, that.matchOnUriPrefix)
+            && Objects.equals(methodRestrict, that.methodRestrict)
+            && Objects.equals(uri, that.uri);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(matchOnUriPrefix, methodRestrict, uri);
+    }
+
 }
diff --git a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowComponent.java b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowComponent.java
index 62014cd..4857b59 100644
--- a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowComponent.java
+++ b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowComponent.java
@@ -19,14 +19,14 @@ package org.apache.camel.component.undertow;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-
 import javax.net.ssl.SSLContext;
 
 import io.undertow.server.HttpHandler;
-
 import org.apache.camel.CamelContext;
 import org.apache.camel.ComponentVerifier;
 import org.apache.camel.Consumer;
@@ -61,7 +61,8 @@ import org.slf4j.LoggerFactory;
 public class UndertowComponent extends DefaultComponent implements RestConsumerFactory, RestApiConsumerFactory, RestProducerFactory, VerifiableComponent, SSLContextParametersAware {
     private static final Logger LOG = LoggerFactory.getLogger(UndertowEndpoint.class);
 
-    private Map<UndertowHostKey, UndertowHost> undertowRegistry = new ConcurrentHashMap<>();
+    private final Map<UndertowHostKey, UndertowHost> undertowRegistry = new ConcurrentHashMap<>();
+    private final Set<HttpHandlerRegistrationInfo> handlers = new HashSet<>();
 
     @Metadata(label = "advanced")
     private UndertowHttpBinding undertowHttpBinding;
@@ -332,6 +333,7 @@ public class UndertowComponent extends DefaultComponent implements RestConsumerF
         final UndertowHost host = undertowRegistry.computeIfAbsent(key, k -> createUndertowHost(k));
 
         host.validateEndpointURI(uri);
+        handlers.add(registrationInfo);
         return host.registerHandler(registrationInfo, handler);
     }
 
@@ -339,6 +341,7 @@ public class UndertowComponent extends DefaultComponent implements RestConsumerF
         final URI uri = registrationInfo.getUri();
         final UndertowHostKey key = new UndertowHostKey(uri.getHost(), uri.getPort(), sslContext);
         final UndertowHost host = undertowRegistry.get(key);
+        handlers.remove(registrationInfo);
         host.unregisterHandler(registrationInfo);
     }
 
@@ -400,4 +403,8 @@ public class UndertowComponent extends DefaultComponent implements RestConsumerF
     protected String getComponentName() {
         return "undertow";
     }
+
+    public Set<HttpHandlerRegistrationInfo> getHandlers() {
+        return handlers;
+    }
 }
diff --git a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowConsumer.java b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowConsumer.java
index 8abc793..4922d24 100644
--- a/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowConsumer.java
+++ b/components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowConsumer.java
@@ -17,7 +17,9 @@
 package org.apache.camel.component.undertow;
 
 import java.io.IOException;
+import java.net.URI;
 import java.nio.ByteBuffer;
+import java.util.Collection;
 
 import io.undertow.Handlers;
 import io.undertow.server.HttpHandler;
@@ -31,6 +33,7 @@ import io.undertow.util.StatusCodes;
 import io.undertow.websockets.core.WebSocketChannel;
 
 import org.apache.camel.AsyncCallback;
+import org.apache.camel.Endpoint;
 import org.apache.camel.Exchange;
 import org.apache.camel.Message;
 import org.apache.camel.Processor;
@@ -38,6 +41,9 @@ import org.apache.camel.TypeConverter;
 import org.apache.camel.component.undertow.UndertowConstants.EventType;
 import org.apache.camel.component.undertow.handlers.CamelWebSocketHandler;
 import org.apache.camel.impl.DefaultConsumer;
+import org.apache.camel.util.CollectionStringBuffer;
+import org.apache.camel.util.ObjectHelper;
+import org.apache.camel.util.StringHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -92,15 +98,30 @@ public class UndertowConsumer extends DefaultConsumer implements HttpHandler {
         HttpString requestMethod = httpExchange.getRequestMethod();
 
         if (Methods.OPTIONS.equals(requestMethod) && !getEndpoint().isOptionsEnabled()) {
-            String allowedMethods;
-            if (getEndpoint().getHttpMethodRestrict() != null) {
-                allowedMethods = getEndpoint().getHttpMethodRestrict();
-                if (!allowedMethods.contains("OPTIONS")) {
-                    allowedMethods = "OPTIONS," + allowedMethods;
+            CollectionStringBuffer csb = new CollectionStringBuffer(",");
+
+            Collection<HttpHandlerRegistrationInfo> handlers = getEndpoint().getComponent().getHandlers();
+            for (HttpHandlerRegistrationInfo reg : handlers) {
+                URI uri = reg.getUri();
+                // what other HTTP methods may exists for the same path
+                if (reg.getMethodRestrict() != null && getEndpoint().getHttpURI().equals(uri)) {
+                    String restrict = reg.getMethodRestrict();
+                    if (restrict.endsWith(",OPTIONS")) {
+                        restrict = restrict.substring(0, restrict.length() - 8);
+                    }
+                    csb.append(restrict);
                 }
-            } else {
+            }
+            String allowedMethods = csb.toString();
+            if (ObjectHelper.isEmpty(allowedMethods)) {
+                allowedMethods = getEndpoint().getHttpMethodRestrict();
+            }
+            if (ObjectHelper.isEmpty(allowedMethods)) {
                 allowedMethods = "GET,HEAD,POST,PUT,DELETE,TRACE,OPTIONS,CONNECT,PATCH";
             }
+            if (!allowedMethods.contains("OPTIONS")) {
+                allowedMethods = allowedMethods + ",OPTIONS";
+            }
             //return list of allowed methods in response headers
             httpExchange.setStatusCode(StatusCodes.OK);
             httpExchange.getResponseHeaders().put(ExchangeHeaders.CONTENT_LENGTH, 0);
diff --git a/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/rest/RestUndertowHttpOptionsTest.java b/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/rest/RestUndertowHttpOptionsTest.java
index ebd7ce8..afbb6b2 100644
--- a/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/rest/RestUndertowHttpOptionsTest.java
+++ b/components/camel-undertow/src/test/java/org/apache/camel/component/undertow/rest/RestUndertowHttpOptionsTest.java
@@ -43,6 +43,19 @@ public class RestUndertowHttpOptionsTest extends BaseUndertowTest {
         assertEquals("", exchange.getIn().getBody(String.class));
     }
 
+    @Test
+    public void testMultipleHttpOptions() {
+        Exchange exchange = template.request("undertow:http://localhost:" + getPort() + "/users/v1/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));
+    }
+
     @Override
     protected RouteBuilder createRouteBuilder() throws Exception {
         return new RouteBuilder() {
@@ -56,7 +69,11 @@ public class RestUndertowHttpOptionsTest extends BaseUndertowTest {
                     .get("v1/customers")
                         .to("mock:customers")
                     .put("v1/{id}")
-                        .to("mock:id");
+                        .to("mock:id")
+                    .get("v1/options")
+                        .to("mock:options")
+                    .post("v1/options")
+                        .to("mock:options");
             }
         };
     }