This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new b090260b85 Locking must check parent collections
b090260b85 is described below
commit b090260b85b9edafc200537080d41cd0292526aa
Author: remm <r...@apache.org>
AuthorDate: Wed Oct 16 10:17:22 2024 +0200
Locking must check parent collections
Code cleanup.
Call if processing in nearly all methods.
Update fixmes.
Update test for collection lock behavior fix.
---
.../apache/catalina/servlets/WebdavServlet.java | 118 +++++++++++++--------
.../catalina/servlets/TestWebdavServlet.java | 63 ++++++-----
webapps/docs/changelog.xml | 4 +
3 files changed, 118 insertions(+), 67 deletions(-)
diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java
b/java/org/apache/catalina/servlets/WebdavServlet.java
index 77cf3b8e1f..15843652bd 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -405,11 +405,13 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
protected boolean checkIfHeaders(HttpServletRequest request,
HttpServletResponse response, WebResource resource)
throws IOException {
- if (!super.checkIfHeaders(request, response, resource)) {
+ // Skip regular HTTP evaluation for a null resource
+ if (resource != null && !super.checkIfHeaders(request, response,
resource)) {
return false;
}
- // TODO : Checking the WebDAV If header
+ // FIXME : Process the WebDAV If header
+
return true;
}
@@ -485,7 +487,7 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
}
// Properties which are to be displayed.
- List<String> properties = null;
+ List<String> properties = new ArrayList<>();
// Propfind depth
int depth = maxDepth;
// Propfind type
@@ -505,8 +507,6 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
}
}
- Node propNode = null;
-
if (req.getContentLengthLong() > 0) {
DocumentBuilder documentBuilder = getDocumentBuilder();
@@ -530,7 +530,22 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
String nodeName = getDAVNode(currentNode);
if ("prop".equals(nodeName)) {
type = FIND_BY_PROPERTY;
- propNode = currentNode;
+ NodeList propChildList =
currentNode.getChildNodes();
+ for (int j = 0; j < propChildList.getLength();
j++) {
+ Node currentNode2 = propChildList.item(j);
+ switch (currentNode2.getNodeType()) {
+ case Node.TEXT_NODE:
+ break;
+ case Node.ELEMENT_NODE:
+ // href is a live property which
is handled differently
+ String propertyName =
getDAVNode(currentNode2);
+ // No support for non DAV:
properties
+ if (propertyName != null) {
+ properties.add(propertyName);
+ }
+ break;
+ }
+ }
}
if ("propname".equals(nodeName)) {
type = FIND_PROPERTY_NAMES;
@@ -548,30 +563,10 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
}
}
- if (type == FIND_BY_PROPERTY) {
- properties = new ArrayList<>();
- // propNode must be non-null if type == FIND_BY_PROPERTY
- @SuppressWarnings("null")
- NodeList childList = propNode.getChildNodes();
-
- for (int i = 0; i < childList.getLength(); i++) {
- Node currentNode = childList.item(i);
- switch (currentNode.getNodeType()) {
- case Node.TEXT_NODE:
- break;
- case Node.ELEMENT_NODE:
- // href is a live property which is handled differently
- String propertyName = getDAVNode(currentNode);
- // No support for non DAV: properties
- if (propertyName != null) {
- properties.add(propertyName);
- }
- break;
- }
- }
- }
-
WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
+ return;
+ }
if (!resource.exists()) {
int slash = path.lastIndexOf('/');
@@ -674,7 +669,7 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
/**
- * PROPPATCH Method.
+ * PROPPATCH Method. Dead properties support is a SHOULD in the
specification and are not implemented.
*
* @param req The Servlet request
* @param resp The Servlet response
@@ -683,18 +678,25 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
*/
protected void doProppatch(HttpServletRequest req, HttpServletResponse
resp) throws IOException {
+ String path = getRelativePath(req);
+
+ WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
+ return;
+ }
+
if (readOnly) {
resp.sendError(WebdavStatus.SC_FORBIDDEN);
return;
}
- if (isLocked(null, req)) {
+ if (isLocked(path, req)) {
resp.sendError(WebdavStatus.SC_LOCKED);
return;
}
- // FIXME
- resp.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED);
+ // FIXME: Compliant PROPPATCH processing
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN);
}
@@ -712,6 +714,9 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
String path = getRelativePath(req);
WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
+ return;
+ }
// Can't create a collection if a resource already exists at the given
// path
@@ -771,6 +776,9 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
}
WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
+ return;
+ }
if (resource.isDirectory()) {
sendNotAllowed(req, resp);
return;
@@ -854,6 +862,10 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
resp.sendError(WebdavStatus.SC_LOCKED);
return;
}
+ WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
+ return;
+ }
LockInfo lock = new LockInfo(maxDepth);
@@ -1034,10 +1046,23 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
lock.path = path;
- WebResource resource = resources.getResource(path);
-
if (lockRequestType == LOCK_CREATION) {
+ // Check if a parent is already locked
+ Iterator<LockInfo> collectionLocksIterator =
collectionLocks.iterator();
+ while (collectionLocksIterator.hasNext()) {
+ LockInfo currentLock = collectionLocksIterator.next();
+ if (currentLock.hasExpired()) {
+ collectionLocksIterator.remove();
+ continue;
+ }
+ if (lock.path.startsWith(currentLock.path + "/") &&
(currentLock.isExclusive() || lock.isExclusive())) {
+ // A parent collection of this collection is locked
+ resp.setStatus(WebdavStatus.SC_LOCKED);
+ return;
+ }
+ }
+
// Generating lock id
String lockTokenStr = req.getServletPath() + "-" + lock.type + "-"
+ lock.scope + "-" +
req.getUserPrincipal() + "-" + lock.depth + "-" +
lock.owner + "-" + lock.tokens + "-" +
@@ -1052,14 +1077,14 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
// Checking if a child resource of this collection is
// already locked
List<String> lockPaths = new ArrayList<>();
- Iterator<LockInfo> collectionLocksIterator =
collectionLocks.iterator();
+ collectionLocksIterator = collectionLocks.iterator();
while (collectionLocksIterator.hasNext()) {
LockInfo currentLock = collectionLocksIterator.next();
if (currentLock.hasExpired()) {
collectionLocksIterator.remove();
continue;
}
- if (currentLock.path.startsWith(lock.path) &&
(currentLock.isExclusive() || lock.isExclusive())) {
+ if (currentLock.path.startsWith(lock.path + "/") &&
(currentLock.isExclusive() || lock.isExclusive())) {
// A child collection of this collection is locked
lockPaths.add(currentLock.path);
}
@@ -1069,7 +1094,7 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
resourceLocks.remove(currentLock.path);
continue;
}
- if (currentLock.path.startsWith(lock.path) &&
(currentLock.isExclusive() || lock.isExclusive())) {
+ if (currentLock.path.startsWith(lock.path + "/") &&
(currentLock.isExclusive() || lock.isExclusive())) {
// A child resource of this collection is locked
lockPaths.add(currentLock.path);
}
@@ -1080,7 +1105,7 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
// One of the child paths was locked
// We generate a multistatus error report
- resp.setStatus(WebdavStatus.SC_CONFLICT);
+ resp.setStatus(WebdavStatus.SC_MULTI_STATUS);
XMLWriter generatedXML = new XMLWriter();
generatedXML.writeXMLHeader();
@@ -1249,9 +1274,8 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
}
String path = getRelativePath(req);
-
- if (isLocked(path, req)) {
- resp.sendError(WebdavStatus.SC_LOCKED);
+ WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
return;
}
@@ -1312,7 +1336,7 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
/**
* Check to see if a resource is currently write locked. The method will
look at the "If" header to make sure the
- * client has give the appropriate lock tokens.
+ * client has demonstrated knowledge of the appropriate lock tokens.
*
* @param path The relative path
* @param req Servlet request
@@ -1331,6 +1355,7 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
ifHeader = "";
}
+ // Normally a lock token is not sent using this header
String lockTokenHeader = req.getHeader("Lock-Token");
if (lockTokenHeader == null) {
lockTokenHeader = "";
@@ -1415,6 +1440,9 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
resp.sendError(WebdavStatus.SC_NOT_FOUND);
return false;
}
+ if (!checkIfHeaders(req, resp, source)) {
+ return false;
+ }
// Parsing destination header
// See RFC 4918
@@ -1663,6 +1691,10 @@ public class WebdavServlet extends DefaultServlet
implements PeriodicEventListen
* @throws IOException If an IO error occurs
*/
private boolean deleteResource(String path, HttpServletRequest req,
HttpServletResponse resp) throws IOException {
+ WebResource resource = resources.getResource(path);
+ if (!checkIfHeaders(req, resp, resource)) {
+ return false;
+ }
return deleteResource(path, req, resp, true);
}
diff --git a/test/org/apache/catalina/servlets/TestWebdavServlet.java
b/test/org/apache/catalina/servlets/TestWebdavServlet.java
index 154305b504..a3d3056a84 100644
--- a/test/org/apache/catalina/servlets/TestWebdavServlet.java
+++ b/test/org/apache/catalina/servlets/TestWebdavServlet.java
@@ -380,7 +380,7 @@ public class TestWebdavServlet extends TomcatBaseTest {
client.setRequest(new String[] { "PUT /myfolder/file4.txt HTTP/1.1" +
SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
"Content-Length: 6" + SimpleHttpClient.CRLF +
- "Lock-Token: " + lockToken + SimpleHttpClient.CRLF +
+ "If: " + lockToken + SimpleHttpClient.CRLF +
"Connection: Close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF + CONTENT });
client.connect();
@@ -397,10 +397,19 @@ public class TestWebdavServlet extends TomcatBaseTest {
client.processRequest(true);
Assert.assertEquals(WebdavStatus.SC_LOCKED, client.getStatusCode());
+ // Unlock /myfolder
+ client.setRequest(new String[] { "UNLOCK /myfolder HTTP/1.1" +
SimpleHttpClient.CRLF +
+ "Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
+ "Lock-Token: " + lockToken + SimpleHttpClient.CRLF +
+ "Connection: Close" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF });
+ client.connect();
+ client.processRequest(true);
+ Assert.assertEquals(HttpServletResponse.SC_NO_CONTENT,
client.getStatusCode());
+
client.setRequest(new String[] { "LOCK /myfolder/file5.txt HTTP/1.1" +
SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
"Content-Length: " + LOCK_BODY.length() +
SimpleHttpClient.CRLF +
- "Lock-Token: " + lockToken + SimpleHttpClient.CRLF +
"Connection: Close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF + LOCK_BODY });
client.connect();
@@ -415,22 +424,11 @@ public class TestWebdavServlet extends TomcatBaseTest {
}
Assert.assertNotNull(lockTokenFile);
- // Try to add /myfolder/file5.txt to myfolder with lock token but
without lock null
- client.setRequest(new String[] { "PUT /myfolder/file5.txt HTTP/1.1" +
SimpleHttpClient.CRLF +
- "Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
- "Content-Length: 6" + SimpleHttpClient.CRLF +
- "Lock-Token: " + lockToken + SimpleHttpClient.CRLF +
- "Connection: Close" + SimpleHttpClient.CRLF +
- SimpleHttpClient.CRLF + CONTENT });
- client.connect();
- client.processRequest(true);
- Assert.assertEquals(WebdavStatus.SC_LOCKED, client.getStatusCode());
-
// Same but with lock token and lock null
client.setRequest(new String[] { "PUT /myfolder/file5.txt HTTP/1.1" +
SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
"Content-Length: 6" + SimpleHttpClient.CRLF +
- "Lock-Token: " + lockToken + ", " + lockTokenFile +
SimpleHttpClient.CRLF +
+ "If: " + lockTokenFile + SimpleHttpClient.CRLF +
"Connection: Close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF + CONTENT });
client.connect();
@@ -441,13 +439,30 @@ public class TestWebdavServlet extends TomcatBaseTest {
client.setRequest(new String[] { "UNLOCK /myfolder/file5.txt HTTP/1.1"
+ SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
"Lock-Token: " + lockTokenFile + SimpleHttpClient.CRLF +
- "If: " + lockToken + SimpleHttpClient.CRLF +
"Connection: Close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF });
client.connect();
client.processRequest(true);
Assert.assertEquals(HttpServletResponse.SC_NO_CONTENT,
client.getStatusCode());
+ // Lock /myfolder again
+ client.setRequest(new String[] { "LOCK /myfolder HTTP/1.1" +
SimpleHttpClient.CRLF +
+ "Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
+ "Content-Length: " + LOCK_BODY.length() +
SimpleHttpClient.CRLF +
+ "Connection: Close" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF + LOCK_BODY });
+ client.connect();
+ client.processRequest(true);
+ Assert.assertEquals(HttpServletResponse.SC_OK, client.getStatusCode());
+
Assert.assertTrue(client.getResponseBody().contains("opaquelocktoken:"));
+ lockToken = null;
+ for (String header : client.getResponseHeaders()) {
+ if (header.startsWith("Lock-Token: ")) {
+ lockToken = header.substring("Lock-Token: ".length());
+ }
+ }
+ Assert.assertNotNull(lockToken);
+
// Copy /myfolder/file5.txt to /myfolder/file6.txt without lock
(should not work)
client.setRequest(new String[] { "COPY /myfolder/file5.txt HTTP/1.1" +
SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
@@ -478,26 +493,26 @@ public class TestWebdavServlet extends TomcatBaseTest {
client.processRequest(true);
Assert.assertEquals(HttpServletResponse.SC_CREATED,
client.getStatusCode());
- // Unlock /myfolder
- client.setRequest(new String[] { "UNLOCK /myfolder HTTP/1.1" +
SimpleHttpClient.CRLF +
+ // Verify that everything created is there
+ client.setRequest(new String[] { "PROPFIND / HTTP/1.1" +
SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
- "Lock-Token: " + lockToken + SimpleHttpClient.CRLF +
"Connection: Close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF });
client.connect();
client.processRequest(true);
- Assert.assertEquals(HttpServletResponse.SC_NO_CONTENT,
client.getStatusCode());
+ Assert.assertEquals(WebdavStatus.SC_MULTI_STATUS,
client.getStatusCode());
+
Assert.assertFalse(client.getResponseBody().contains("/myfolder/file4.txt"));
+ Assert.assertTrue(client.getResponseBody().contains("/file7.txt"));
- // Verify that everything created is there
- client.setRequest(new String[] { "PROPFIND / HTTP/1.1" +
SimpleHttpClient.CRLF +
+ // Unlock /myfolder again
+ client.setRequest(new String[] { "UNLOCK /myfolder HTTP/1.1" +
SimpleHttpClient.CRLF +
"Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
+ "Lock-Token: " + lockToken + SimpleHttpClient.CRLF +
"Connection: Close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF });
client.connect();
client.processRequest(true);
- Assert.assertEquals(WebdavStatus.SC_MULTI_STATUS,
client.getStatusCode());
-
Assert.assertFalse(client.getResponseBody().contains("/myfolder/file4.txt"));
- Assert.assertTrue(client.getResponseBody().contains("/file7.txt"));
+ Assert.assertEquals(HttpServletResponse.SC_NO_CONTENT,
client.getStatusCode());
// Delete /myfolder
client.setRequest(new String[] { "DELETE /myfolder HTTP/1.1" +
SimpleHttpClient.CRLF +
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 955440c8ee..d94984606e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -167,6 +167,10 @@
<fix>
Enforce <code>DAV:</code> namespace on WebDAV XML elements. (remm)
</fix>
+ <fix>
+ Do not allow a new WebDAV lock on a child resource if a parent
+ collection is locked (RFC 4918 section 6.1). (remm)
+ </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
