You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by ff...@apache.org on 2018/07/04 09:47:10 UTC

[camel] branch master updated: [CAMEL-12621]Rest DSL with netty4-http components returns 404 instead of 405, when http method is not supported

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 216aa96  [CAMEL-12621]Rest DSL with netty4-http components returns 404 instead of 405, when http method is not supported
216aa96 is described below

commit 216aa96eb11885a77d9506a5cd07f0f834acc1ce
Author: Freeman Fang <fr...@gmail.com>
AuthorDate: Wed Jul 4 17:46:40 2018 +0800

    [CAMEL-12621]Rest DSL with netty4-http components returns 404 instead of 405, when http method is not supported
---
 .../HttpServerMultiplexChannelHandler.java         | 26 +++++++--
 .../http/rest/RestNettyMethodNotAllowedTest.java   | 67 ++++++++++++++++++++++
 2 files changed, 88 insertions(+), 5 deletions(-)

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 abab702..3f5f1a1 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
@@ -22,6 +22,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArraySet;
 
+
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandler.Sharable;
 import io.netty.channel.ChannelHandlerContext;
@@ -35,11 +36,14 @@ import io.netty.util.AttributeKey;
 import org.apache.camel.Exchange;
 import org.apache.camel.component.netty4.http.HttpServerConsumerChannelFactory;
 import org.apache.camel.component.netty4.http.NettyHttpConsumer;
+import org.apache.camel.http.common.CamelServlet;
+import org.apache.camel.http.common.HttpConsumer;
 import org.apache.camel.support.RestConsumerContextPathMatcher;
 import org.apache.camel.util.UnsafeUriCharactersEncoder;
 import org.slf4j.Logger;
 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.HttpVersion.HTTP_1_1;
 
@@ -100,7 +104,7 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
       
         LOG.debug("Message received: {}", request);
 
-        HttpServerChannelHandler handler = getHandler(request);
+        HttpServerChannelHandler handler = getHandler(request, request.method().name());
         if (handler != null) {
             Attribute<HttpServerChannelHandler> attr = ctx.channel().attr(SERVER_HANDLER_KEY);
             // store handler as attachment
@@ -112,8 +116,17 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
             }   
             handler.channelRead(ctx, request);
         } else {
-            // this resource is not found, so send empty response back
-            HttpResponse response = new DefaultHttpResponse(HTTP_1_1, NOT_FOUND);
+            // 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
+            boolean hasAnyMethod = CamelServlet.METHODS.stream().anyMatch(m -> isHttpMethodAllowed(request, m));
+            HttpResponse response = null;
+            if (hasAnyMethod) {
+                //method match error, return 405
+                response = new DefaultHttpResponse(HTTP_1_1, METHOD_NOT_ALLOWED);
+            } else {
+                // this resource is not found, return 404
+                response = new DefaultHttpResponse(HTTP_1_1, NOT_FOUND);
+            }
             response.headers().set(Exchange.CONTENT_TYPE, "text/plain");
             response.headers().set(Exchange.CONTENT_LENGTH, 0);
             ctx.writeAndFlush(response);
@@ -145,12 +158,15 @@ public class HttpServerMultiplexChannelHandler extends SimpleChannelInboundHandl
         }
     }
 
+    private boolean isHttpMethodAllowed(HttpRequest request, String method) {
+        return getHandler(request, method) != null;
+    }
+    
     @SuppressWarnings("unchecked")
-    private HttpServerChannelHandler getHandler(HttpRequest request) {
+    private HttpServerChannelHandler getHandler(HttpRequest request, String method) {
         HttpServerChannelHandler answer = null;
 
         // need to strip out host and port etc, as we only need the context-path for matching
-        String method = request.method().name();
         if (method == null) {
             return null;
         }
diff --git a/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyMethodNotAllowedTest.java b/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyMethodNotAllowedTest.java
new file mode 100644
index 0000000..f2c08a8
--- /dev/null
+++ b/components/camel-netty4-http/src/test/java/org/apache/camel/component/netty4/http/rest/RestNettyMethodNotAllowedTest.java
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.netty4.http.rest;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.netty4.http.BaseNettyTest;
+import org.apache.camel.http.common.HttpOperationFailedException;
+import org.junit.Test;
+
+public class RestNettyMethodNotAllowedTest extends BaseNettyTest {
+    @Test
+    public void testMethodNotAllowed() {
+        try {
+            template.sendBody("http://localhost:" + getPort() + "/users/123/basic", "body");
+            fail("Shall not pass!");
+        } catch (Exception e) {
+            HttpOperationFailedException hofe = assertIsInstanceOf(HttpOperationFailedException.class, e.getCause());
+            assertEquals(405, hofe.getStatusCode());
+        }
+    }
+    
+    @Test
+    public void testMethodAllowed() {
+        try {
+            template.sendBodyAndHeader("http://localhost:" + getPort() + "/users/123/basic", "body", Exchange.HTTP_METHOD, "GET");
+        } catch (Exception e) {
+            fail("Shall pass with GET http method!");
+        }
+    }
+
+    @Override
+    protected RouteBuilder createRouteBuilder() throws Exception {
+        return new RouteBuilder() {
+            @Override
+            public void configure() {
+                // configure to use netty on localhost
+                restConfiguration().component("netty4-http").host("localhost").port(getPort());
+
+                // use the rest DSL to define the rest services
+                rest("/users/")
+                        .get("{id}/basic")
+                        .route()
+                        .to("mock:input")
+                        .process(exchange -> {
+                            String id = exchange.getIn().getHeader("id", String.class);
+                            exchange.getOut().setBody(id + ";Donald Duck");
+                        });
+            }
+        };
+    }
+
+}