Author: markt
Date: Tue Jan 9 13:50:52 2018
New Revision: 1820663
URL: http://svn.apache.org/viewvc?rev=1820663&view=rev
Log:
Modify the Default and WebDAV Servlets so that a 405 status code is returned
for PUT and DELETE requests when disabled via the readonly initialisation
parameter.
Align the contents of the <code>Allow</code> header with the response code for
the Default and WebDAV Servlets. For any given resource a method that returns a
405 status code will not be listed in the Allow header and a method listed in
the Allow header will not return a 405 status code.
Based on a patch suggested by Ken Dombeck.
This closes #96
Added:
tomcat/trunk/test/org/apache/catalina/servlets/ServletOptionsBaseTest.java
(with props)
tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServletOptions.java
(with props)
tomcat/trunk/test/org/apache/catalina/servlets/TestWebdavServletOptions.java
(with props)
Modified:
tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java
tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1820663&r1=1820662&r2=1820663&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java Tue Jan
9 13:50:52 2018
@@ -499,24 +499,35 @@ public class DefaultServlet extends Http
protected void doOptions(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
+ resp.setHeader("Allow", determineMethodsAllowed(req));
+ }
+
+
+ protected String determineMethodsAllowed(HttpServletRequest req) {
StringBuilder allow = new StringBuilder();
- // There is a doGet method
- allow.append("GET, HEAD");
- // There is a doPost
- allow.append(", POST");
- // There is a doPut
- allow.append(", PUT");
- // There is a doDelete
- allow.append(", DELETE");
+
+ // Start with methods that are always allowed
+ allow.append("OPTIONS, GET, HEAD, POST");
+
+ // PUT and DELETE depend on readonly
+ if (!readOnly) {
+ allow.append(", PUT, DELETE");
+ }
+
// Trace - assume disabled unless we can prove otherwise
if (req instanceof RequestFacade &&
((RequestFacade) req).getAllowTrace()) {
allow.append(", TRACE");
}
- // Always allow options
- allow.append(", OPTIONS");
- resp.setHeader("Allow", allow.toString());
+ return allow.toString();
+ }
+
+
+ protected void sendNotAllowed(HttpServletRequest req, HttpServletResponse
resp)
+ throws IOException {
+ resp.addHeader("Allow", determineMethodsAllowed(req));
+ resp.sendError(WebdavStatus.SC_METHOD_NOT_ALLOWED);
}
@@ -551,7 +562,7 @@ public class DefaultServlet extends Http
throws ServletException, IOException {
if (readOnly) {
- resp.sendError(HttpServletResponse.SC_FORBIDDEN);
+ sendNotAllowed(req, resp);
return;
}
@@ -675,7 +686,7 @@ public class DefaultServlet extends Http
throws ServletException, IOException {
if (readOnly) {
- resp.sendError(HttpServletResponse.SC_FORBIDDEN);
+ sendNotAllowed(req, resp);
return;
}
Modified: tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java?rev=1820663&r1=1820662&r2=1820663&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/WebdavServlet.java Tue Jan
9 13:50:52 2018
@@ -464,10 +464,7 @@ public class WebdavServlet extends Defau
throws ServletException, IOException {
resp.addHeader("DAV", "1,2");
-
- StringBuilder methodsAllowed = determineMethodsAllowed(req);
-
- resp.addHeader("Allow", methodsAllowed.toString());
+ resp.addHeader("Allow", determineMethodsAllowed(req));
resp.addHeader("MS-Author-Via", "DAV");
}
@@ -483,11 +480,7 @@ public class WebdavServlet extends Defau
throws ServletException, IOException {
if (!listings) {
- // Get allowed methods
- StringBuilder methodsAllowed = determineMethodsAllowed(req);
-
- resp.addHeader("Allow", methodsAllowed.toString());
- resp.sendError(WebdavStatus.SC_METHOD_NOT_ALLOWED);
+ sendNotAllowed(req, resp);
return;
}
@@ -743,16 +736,6 @@ public class WebdavServlet extends Defau
protected void doMkcol(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
- if (readOnly) {
- resp.sendError(WebdavStatus.SC_FORBIDDEN);
- return;
- }
-
- if (isLocked(req)) {
- resp.sendError(WebdavStatus.SC_LOCKED);
- return;
- }
-
String path = getRelativePath(req);
WebResource resource = resources.getResource(path);
@@ -760,12 +743,17 @@ public class WebdavServlet extends Defau
// Can't create a collection if a resource already exists at the given
// path
if (resource.exists()) {
- // Get allowed methods
- StringBuilder methodsAllowed = determineMethodsAllowed(req);
+ sendNotAllowed(req, resp);
+ return;
+ }
- resp.addHeader("Allow", methodsAllowed.toString());
+ if (readOnly) {
+ resp.sendError(WebdavStatus.SC_FORBIDDEN);
+ return;
+ }
- resp.sendError(WebdavStatus.SC_METHOD_NOT_ALLOWED);
+ if (isLocked(req)) {
+ resp.sendError(WebdavStatus.SC_LOCKED);
return;
}
@@ -809,7 +797,7 @@ public class WebdavServlet extends Defau
throws ServletException, IOException {
if (readOnly) {
- resp.sendError(WebdavStatus.SC_FORBIDDEN);
+ sendNotAllowed(req, resp);
return;
}
@@ -841,9 +829,14 @@ public class WebdavServlet extends Defau
return;
}
- super.doPut(req, resp);
-
String path = getRelativePath(req);
+ WebResource resource = resources.getResource(path);
+ if (resource.isDirectory()) {
+ sendNotAllowed(req, resp);
+ return;
+ }
+
+ super.doPut(req, resp);
// Removing any lock-null resource which would be present
lockNullResources.remove(path);
@@ -2325,41 +2318,50 @@ public class WebdavServlet extends Defau
/**
* Determines the methods normally allowed for the resource.
+ *
* @param req The Servlet request
- * @return a string builder with the allowed HTTP methods
+ *
+ * @return The allowed HTTP methods
*/
- private StringBuilder determineMethodsAllowed(HttpServletRequest req) {
+ @Override
+ protected String determineMethodsAllowed(HttpServletRequest req) {
- StringBuilder methodsAllowed = new StringBuilder();
WebResource resource = resources.getResource(getRelativePath(req));
- if (!resource.exists()) {
- methodsAllowed.append("OPTIONS, MKCOL, PUT, LOCK");
- return methodsAllowed;
+ // These methods are always allowed. They may return a 404 (not a 405)
+ // if the resource does not exist.
+ StringBuilder methodsAllowed = new StringBuilder(
+ "OPTIONS, GET, POST, HEAD");
+
+ if (!readOnly) {
+ methodsAllowed.append(", DELETE");
+ if (!resource.isDirectory()) {
+ methodsAllowed.append(", PUT");
+ }
}
- methodsAllowed.append("OPTIONS, GET, HEAD, POST, DELETE");
// Trace - assume disabled unless we can prove otherwise
if (req instanceof RequestFacade &&
((RequestFacade) req).getAllowTrace()) {
methodsAllowed.append(", TRACE");
}
- methodsAllowed.append(", PROPPATCH, COPY, MOVE, LOCK, UNLOCK");
+
+ methodsAllowed.append(", LOCK, UNLOCK, PROPPATCH, COPY, MOVE");
if (listings) {
methodsAllowed.append(", PROPFIND");
}
- if (resource.isFile()) {
- methodsAllowed.append(", PUT");
+ if (!resource.exists()) {
+ methodsAllowed.append(", MKCOL");
}
- return methodsAllowed;
+ return methodsAllowed.toString();
}
- // -------------------------------------------------- LockInfo Inner Class
+ // -------------------------------------------------- LockInfo Inner Class
/**
* Holds a lock information.
Added:
tomcat/trunk/test/org/apache/catalina/servlets/ServletOptionsBaseTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/servlets/ServletOptionsBaseTest.java?rev=1820663&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/servlets/ServletOptionsBaseTest.java
(added)
+++ tomcat/trunk/test/org/apache/catalina/servlets/ServletOptionsBaseTest.java
Tue Jan 9 13:50:52 2018
@@ -0,0 +1,161 @@
+/*
+ * 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.catalina.servlets;
+
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.servlet.Servlet;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runners.Parameterized.Parameter;
+
+import static org.apache.catalina.startup.SimpleHttpClient.CRLF;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+public abstract class ServletOptionsBaseTest extends TomcatBaseTest {
+
+ protected static final String COLLECTION_NAME = "collection";
+ protected static final String FILE_NAME = "file";
+ protected static final String UNKNOWN_NAME = "unknown";
+
+ @Parameter(0)
+ public boolean listings;
+
+ @Parameter(1)
+ public boolean readonly;
+
+ @Parameter(2)
+ public boolean trace;
+
+ @Parameter(3)
+ public String url;
+
+ @Parameter(4)
+ public String method;
+
+
+ /*
+ * Check that methods returned by OPTIONS are consistent with the return
+ * http status code.
+ * Method not present in options response -> 405 expected
+ * Method present in options response -> anything other than 405
expected
+ */
+ @Test
+ public void testOptions() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ tomcat.getConnector().setAllowTrace(trace);
+
+ File docBase = new File(getTemporaryDirectory(), "webdav");
+ File collection = new File(docBase, COLLECTION_NAME);
+ Assert.assertTrue(collection.mkdirs());
+ File file = new File(docBase, FILE_NAME);
+ Assert.assertTrue(file.createNewFile());
+
+ addDeleteOnTearDown(docBase);
+
+ // app dir is relative to server home
+ org.apache.catalina.Context ctx =
+ tomcat.addWebapp(null, "/servlet", docBase.getAbsolutePath());
+
+ Wrapper w = Tomcat.addServlet(ctx, "servlet", createServlet());
+ w.addInitParameter("listings", Boolean.toString(listings));
+ w.addInitParameter("readonly", Boolean.toString(readonly));
+
+ ctx.addServletMappingDecoded("/*", "servlet");
+
+ tomcat.start();
+
+ OptionsHttpClient client = new OptionsHttpClient();
+ client.setPort(getPort());
+ client.setRequest(new String[] {
+ "OPTIONS /servlet/" + url + " HTTP/1.1" + CRLF +
+ "Host: localhost:" + getPort() + CRLF +
+ "Connection: close" + CRLF +
+ CRLF });
+
+ client.connect();
+ client.processRequest();
+
+ Assert.assertTrue(client.isResponse200());
+ Set<String> allowed = client.getAllowedMethods();
+
+ client.disconnect();
+ client.reset();
+
+ client.setRequest(new String[] {
+ method + " /servlet/" + url + " HTTP/1.1" + CRLF +
+ "Host: localhost:" + getPort() + CRLF +
+ "Connection: close" + CRLF +
+ CRLF });
+
+ client.connect();
+ client.processRequest();
+
+ String msg = "Listings[" + listings + "], readonly [" + readonly +
+ "], trace[ " + trace + "], url[" + url + "], method[" + method
+ "]";
+
+ Assert.assertNotNull(client.getResponseLine());
+
+ if (allowed.contains(method)) {
+ Assert.assertFalse(msg, client.isResponse405());
+ } else {
+ Assert.assertTrue(msg, client.isResponse405());
+ allowed = client.getAllowedMethods();
+ Assert.assertFalse(msg, allowed.contains(method));
+ }
+ }
+
+
+ protected abstract Servlet createServlet();
+
+
+ private static class OptionsHttpClient extends SimpleHttpClient {
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return true;
+ }
+
+ public Set<String> getAllowedMethods() {
+ String valueList = null;
+ for (String header : getResponseHeaders()) {
+ if (header.startsWith("Allow:")) {
+ valueList = header.substring(6).trim();
+ break;
+ }
+ }
+ Assert.assertNotNull(valueList);
+ String[] values = valueList.split(",");
+ for (int i = 0; i < values.length; i++) {
+ values[i] = values[i].trim();
+ }
+ Set<String> allowed = new HashSet<>();
+ allowed.addAll(Arrays.asList(values));
+
+ return allowed;
+ }
+ }
+}
Propchange:
tomcat/trunk/test/org/apache/catalina/servlets/ServletOptionsBaseTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Added:
tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServletOptions.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServletOptions.java?rev=1820663&view=auto
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServletOptions.java
(added)
+++
tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServletOptions.java
Tue Jan 9 13:50:52 2018
@@ -0,0 +1,61 @@
+/*
+ * 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.catalina.servlets;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import javax.servlet.Servlet;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class TestDefaultServletOptions extends ServletOptionsBaseTest {
+
+ @Parameters
+ public static Collection<Object[]> inputs() {
+ Boolean[] booleans = new Boolean[] { Boolean.FALSE, Boolean.TRUE };
+ String[] urls = new String[] { COLLECTION_NAME, FILE_NAME,
UNKNOWN_NAME };
+ String[] methods = new String[] { "GET", "POST", "HEAD", "TRACE",
"PUT", "DELETE" };
+
+ List<Object[]> result = new ArrayList<>();
+
+ for (Boolean listingsValue : booleans) {
+ for (Boolean readOnlyValue : booleans) {
+ for (Boolean traceValue : booleans) {
+ for (String url : urls) {
+ for (String method : methods) {
+ result.add(new Object[] {
+ listingsValue, readOnlyValue, traceValue,
url, method } );
+ }
+ }
+ }
+ }
+
+ }
+ return result;
+ }
+
+
+ @Override
+ protected Servlet createServlet() {
+ return new DefaultServlet();
+ }
+}
Propchange:
tomcat/trunk/test/org/apache/catalina/servlets/TestDefaultServletOptions.java
------------------------------------------------------------------------------
svn:eol-style = native
Added:
tomcat/trunk/test/org/apache/catalina/servlets/TestWebdavServletOptions.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/servlets/TestWebdavServletOptions.java?rev=1820663&view=auto
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/servlets/TestWebdavServletOptions.java
(added)
+++
tomcat/trunk/test/org/apache/catalina/servlets/TestWebdavServletOptions.java
Tue Jan 9 13:50:52 2018
@@ -0,0 +1,62 @@
+/*
+ * 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.catalina.servlets;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import javax.servlet.Servlet;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class TestWebdavServletOptions extends ServletOptionsBaseTest {
+
+ @Parameters
+ public static Collection<Object[]> inputs() {
+ Boolean[] booleans = new Boolean[] { Boolean.FALSE, Boolean.TRUE };
+ String[] urls = new String[] { COLLECTION_NAME, FILE_NAME,
UNKNOWN_NAME };
+ String[] methods = new String[] { "GET", "POST", "HEAD", "TRACE",
"PUT", "DELETE",
+ "MKCOL", "LOCK", "UNLOCK", "COPY", "MOVE", "PROPFIND",
"PROPPATCH" };
+
+ List<Object[]> result = new ArrayList<>();
+
+ for (Boolean listingsValue : booleans) {
+ for (Boolean readOnlyValue : booleans) {
+ for (Boolean traceValue : booleans) {
+ for (String url : urls) {
+ for (String method : methods) {
+ result.add(new Object[] {
+ listingsValue, readOnlyValue, traceValue,
url, method } );
+ }
+ }
+ }
+ }
+
+ }
+ return result;
+ }
+
+
+ @Override
+ protected Servlet createServlet() {
+ return new WebdavServlet();
+ }
+}
Propchange:
tomcat/trunk/test/org/apache/catalina/servlets/TestWebdavServletOptions.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java?rev=1820663&r1=1820662&r2=1820663&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java Tue Jan
9 13:50:52 2018
@@ -54,6 +54,7 @@ public abstract class SimpleHttpClient {
public static final String REDIRECT_303 = "HTTP/1.1 303 ";
public static final String FAIL_400 = "HTTP/1.1 400 ";
public static final String FAIL_404 = "HTTP/1.1 404 ";
+ public static final String FAIL_405 = "HTTP/1.1 405 ";
public static final String TIMEOUT_408 = "HTTP/1.1 408 ";
public static final String FAIL_413 = "HTTP/1.1 413 ";
public static final String FAIL_417 = "HTTP/1.1 417 ";
@@ -429,6 +430,10 @@ public abstract class SimpleHttpClient {
return responseLineStartsWith(FAIL_404);
}
+ public boolean isResponse405() {
+ return responseLineStartsWith(FAIL_405);
+ }
+
public boolean isResponse408() {
return responseLineStartsWith(TIMEOUT_408);
}
Modified: tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1820663&r1=1820662&r2=1820663&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java (original)
+++ tomcat/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java Tue Jan
9 13:50:52 2018
@@ -74,9 +74,9 @@ import org.apache.tomcat.util.scan.Stand
* don't have to keep writing the cleanup code.
*/
public abstract class TomcatBaseTest extends LoggingBaseTest {
- private static final int DEFAULT_CLIENT_TIMEOUT_MS = 300_000;
private Tomcat tomcat;
private boolean accessLogEnabled = false;
+ protected static final int DEFAULT_CLIENT_TIMEOUT_MS = 300_000;
public static final String TEMP_DIR = System.getProperty("java.io.tmpdir");
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1820663&r1=1820662&r2=1820663&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Jan 9 13:50:52 2018
@@ -73,6 +73,18 @@
rather than fails (with a 500 response). This enables Tomcat to pass
two
additional tests from the Litmus WebDAV test suite. (markt)
</fix>
+ <update>
+ Modify the Default and WebDAV Servlets so that a 405 status code is
+ returned for <code>PUT</code> and <code>DELETE</code> requests when
+ disabled via the <code>readonly</code> initialisation parameter.
+ </update>
+ <fix>
+ Align the contents of the <code>Allow</code> header with the response
+ code for the Default and WebDAV Servlets. For any given resource a
+ method that returns a 405 status code will not be listed in the
+ <code>Allow</code> header and a method listed in the <code>Allow</code>
+ header will not return a 405 status code. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
