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
The following commit(s) were added to refs/heads/master by this push:
new f0c72f1 CAMEL-12050: camel-servlet/camel-jetty etc should return 405
error if the path exists but the http method is not allowed.
f0c72f1 is described below
commit f0c72f1b573afd472e60dd90372f4a536b026d91
Author: Claus Ibsen <claus.ib...@gmail.com>
AuthorDate: Thu Jan 11 19:21:29 2018 +0100
CAMEL-12050: camel-servlet/camel-jetty etc should return 405 error if the
path exists but the http method is not allowed.
---
.../org/apache/camel/http/common/CamelServlet.java | 22 +++++--
.../HttpRestServletResolveConsumerStrategy.java | 13 +---
.../common/HttpServletResolveConsumerStrategy.java | 51 +++++++++------
.../common/ServletResolveConsumerStrategy.java | 11 ++++
.../rest/RestServletMethodNotAllowedTest.java | 73 ++++++++++++++++++++++
5 files changed, 137 insertions(+), 33 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 2bfcaba..19755b4 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
@@ -17,7 +17,9 @@
package org.apache.camel.http.common;
import java.io.IOException;
+import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -40,9 +42,12 @@ import org.slf4j.LoggerFactory;
*/
public class CamelServlet extends HttpServlet {
public static final String ASYNC_PARAM = "async";
+
private static final long serialVersionUID = -7061982839117697829L;
+ private static final List<String> METHODS = Arrays.asList("GET", "HEAD",
"POST", "PUT", "DELETE", "TRACE", "OPTIONS", "CONNECT", "PATCH");
+
protected final Logger log = LoggerFactory.getLogger(getClass());
-
+
/**
* We have to define this explicitly so the name can be set as we can not
always be
* sure that it is already set via the init method
@@ -113,9 +118,18 @@ public class CamelServlet extends HttpServlet {
// Is there a consumer registered for the request.
HttpConsumer consumer = resolve(request);
if (consumer == null) {
- log.debug("No consumer to service request {}", request);
- response.sendError(HttpServletResponse.SC_NOT_FOUND);
- return;
+ // 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 = METHODS.stream().anyMatch(m ->
getServletResolveConsumerStrategy().isHttpMethodAllowed(request, m,
getConsumers()));
+ if (hasAnyMethod) {
+ log.debug("No consumer to service request {} as method {} is
not allowed", request, request.getMethod());
+ response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
+ return;
+ } else {
+ log.debug("No consumer to service request {} as resource is
not found", request);
+ response.sendError(HttpServletResponse.SC_NOT_FOUND);
+ return;
+ }
}
// are we suspended?
diff --git
a/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpRestServletResolveConsumerStrategy.java
b/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpRestServletResolveConsumerStrategy.java
index ae412b3..83cbcd4 100644
---
a/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpRestServletResolveConsumerStrategy.java
+++
b/components/camel-http-common/src/main/java/org/apache/camel/http/common/HttpRestServletResolveConsumerStrategy.java
@@ -29,19 +29,13 @@ import
org.apache.camel.support.RestConsumerContextPathMatcher;
public class HttpRestServletResolveConsumerStrategy extends
HttpServletResolveConsumerStrategy {
@Override
- @SuppressWarnings("unchecked")
- public HttpConsumer resolve(HttpServletRequest request, Map<String,
HttpConsumer> consumers) {
+ protected HttpConsumer doResolve(HttpServletRequest request, String
method, Map<String, HttpConsumer> consumers) {
HttpConsumer answer = null;
String path = request.getPathInfo();
if (path == null) {
return null;
}
- String method = request.getMethod();
- if (method == null) {
- return null;
- }
-
List<RestConsumerContextPathMatcher.ConsumerPath> paths = new
ArrayList<>();
for (final Map.Entry<String, HttpConsumer> entry :
consumers.entrySet()) {
paths.add(new HttpRestConsumerPath(entry.getValue()));
@@ -54,10 +48,9 @@ public class HttpRestServletResolveConsumerStrategy extends
HttpServletResolveCo
if (answer == null) {
// fallback to default
- answer = super.resolve(request, consumers);
+ answer = super.doResolve(request, method, consumers);
}
return answer;
}
-
-}
\ No newline at end of file
+}
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 e55e650..e08827a 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
@@ -32,39 +32,52 @@ public class HttpServletResolveConsumerStrategy implements
ServletResolveConsume
@Override
public HttpConsumer resolve(HttpServletRequest request, Map<String,
HttpConsumer> consumers) {
+ return doResolve(request, request.getMethod(), consumers);
+ }
+
+ @Override
+ public boolean isHttpMethodAllowed(HttpServletRequest request, String
method, Map<String, HttpConsumer> consumers) {
+ return doResolve(request, method, consumers) != null;
+ }
+
+ protected HttpConsumer doResolve(HttpServletRequest request, String
method, Map<String, HttpConsumer> consumers) {
String path = request.getPathInfo();
if (path == null) {
return null;
}
HttpConsumer answer = consumers.get(path);
- if (answer == null) {
- List<HttpConsumer> candidates = new ArrayList<>();
- for (String key : consumers.keySet()) {
- //We need to look up the consumer path here
- String consumerPath = consumers.get(key).getPath();
- HttpConsumer consumer = consumers.get(key);
- boolean matchOnUriPrefix =
consumer.getEndpoint().isMatchOnUriPrefix();
- // Just make sure the we get the right consumer path first
- if (RestConsumerContextPathMatcher.matchPath(path,
consumerPath, matchOnUriPrefix)) {
- candidates.add(consumer);
- }
- }
-
+ List<HttpConsumer> candidates = resolveCandidates(request, method,
consumers);
+ 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);
- } else {
- // extra filter by restrict
- candidates = candidates.stream().filter(c ->
matchRestMethod(request.getMethod(),
c.getEndpoint().getHttpMethodRestrict())).collect(Collectors.toList());
- if (candidates.size() == 1) {
- answer = candidates.get(0);
- }
}
}
return answer;
}
+ private List<HttpConsumer> resolveCandidates(HttpServletRequest request,
String method, Map<String, HttpConsumer> consumers) {
+ String path = request.getPathInfo();
+
+ List<HttpConsumer> candidates = new ArrayList<>();
+ for (String key : consumers.keySet()) {
+ //We need to look up the consumer path here
+ String consumerPath = consumers.get(key).getPath();
+ HttpConsumer consumer = consumers.get(key);
+ boolean matchOnUriPrefix =
consumer.getEndpoint().isMatchOnUriPrefix();
+ // Just make sure the we get the right consumer path first
+ if (RestConsumerContextPathMatcher.matchPath(path, consumerPath,
matchOnUriPrefix)) {
+ candidates.add(consumer);
+ }
+ }
+ return candidates;
+ }
+
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-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 a16df19..039f74b 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
@@ -34,4 +34,15 @@ public interface ServletResolveConsumerStrategy {
*/
HttpConsumer resolve(HttpServletRequest request, Map<String, HttpConsumer>
consumers);
+ /**
+ * Checks if the http request method (GET, POST, etc) would be allow among
the registered consumers.
+
+ * @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.
+ */
+ boolean isHttpMethodAllowed(HttpServletRequest request, String method,
Map<String, HttpConsumer> consumers);
+
}
diff --git
a/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletMethodNotAllowedTest.java
b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletMethodNotAllowedTest.java
new file mode 100644
index 0000000..d3a1323
--- /dev/null
+++
b/components/camel-servlet/src/test/java/org/apache/camel/component/servlet/rest/RestServletMethodNotAllowedTest.java
@@ -0,0 +1,73 @@
+/**
+ * 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.servlet.rest;
+
+import com.meterware.httpunit.PostMethodWebRequest;
+import com.meterware.httpunit.WebRequest;
+import com.meterware.httpunit.WebResponse;
+import com.meterware.servletunit.ServletUnitClient;
+import org.apache.camel.Exchange;
+import org.apache.camel.Processor;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.servlet.ServletCamelRouterTestSupport;
+import org.apache.camel.component.servlet.ServletRestHttpBinding;
+import org.apache.camel.impl.JndiRegistry;
+import org.junit.Test;
+
+public class RestServletMethodNotAllowedTest extends
ServletCamelRouterTestSupport {
+
+ @Override
+ protected JndiRegistry createRegistry() throws Exception {
+ JndiRegistry jndi = super.createRegistry();
+ jndi.bind("myBinding", new ServletRestHttpBinding());
+ return jndi;
+ }
+
+ @Test
+ public void testServletMethodNotAllowed() throws Exception {
+ WebRequest req = new PostMethodWebRequest(CONTEXT_URL +
"/services/users/123/basic");
+ ServletUnitClient client = newClient();
+ client.setExceptionsThrownOnErrorStatus(false);
+ WebResponse response = client.getResponse(req);
+
+ assertEquals(405, response.getResponseCode());
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() throws Exception {
+ return new RouteBuilder() {
+ @Override
+ public void configure() throws Exception {
+ // configure to use servlet on localhost
+
restConfiguration().component("servlet").host("localhost").endpointProperty("httpBinding",
"#myBinding");
+
+ // use the rest DSL to define the rest services
+ rest("/users/")
+ .get("{id}/basic")
+ .route()
+ .to("mock:input")
+ .process(new Processor() {
+ public void process(Exchange exchange) throws
Exception {
+ String id = exchange.getIn().getHeader("id",
String.class);
+ exchange.getOut().setBody(id + ";Donald Duck");
+ }
+ });
+ }
+ };
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
['"commits@camel.apache.org" <commits@camel.apache.org>'].
