Skip to content

Commit

Permalink
Modify the Default and WebDAV Servlets so that a 405 status code is r…
Browse files Browse the repository at this point in the history
…eturned 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

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1820663 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
markt-asf committed Jan 9, 2018
1 parent fca4ac9 commit 18ad6f3
Show file tree
Hide file tree
Showing 8 changed files with 366 additions and 52 deletions.
37 changes: 24 additions & 13 deletions java/org/apache/catalina/servlets/DefaultServlet.java
Expand Up @@ -499,24 +499,35 @@ protected void doHead(HttpServletRequest request, HttpServletResponse response)
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);
}


Expand Down Expand Up @@ -551,7 +562,7 @@ protected void doPut(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {

if (readOnly) {
resp.sendError(HttpServletResponse.SC_FORBIDDEN);
sendNotAllowed(req, resp);
return;
}

Expand Down Expand Up @@ -675,7 +686,7 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {

if (readOnly) {
resp.sendError(HttpServletResponse.SC_FORBIDDEN);
sendNotAllowed(req, resp);
return;
}

Expand Down
78 changes: 40 additions & 38 deletions java/org/apache/catalina/servlets/WebdavServlet.java
Expand Up @@ -464,10 +464,7 @@ protected void doOptions(HttpServletRequest req, HttpServletResponse resp)
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");
}

Expand All @@ -483,11 +480,7 @@ protected void doPropfind(HttpServletRequest req, HttpServletResponse resp)
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;
}

Expand Down Expand Up @@ -743,29 +736,24 @@ protected void doProppatch(HttpServletRequest req, HttpServletResponse resp)
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);

// 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;
}

Expand Down Expand Up @@ -809,7 +797,7 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {

if (readOnly) {
resp.sendError(WebdavStatus.SC_FORBIDDEN);
sendNotAllowed(req, resp);
return;
}

Expand Down Expand Up @@ -841,9 +829,14 @@ protected void doPut(HttpServletRequest req, HttpServletResponse resp)
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);
Expand Down Expand Up @@ -2325,41 +2318,50 @@ private String getISOCreationDate(long creationDate) {

/**
* 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.
Expand Down
161 changes: 161 additions & 0 deletions test/org/apache/catalina/servlets/ServletOptionsBaseTest.java
@@ -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;
}
}
}

0 comments on commit 18ad6f3

Please sign in to comment.