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 744db231c2 Improve lock test coverage a bit more 744db231c2 is described below commit 744db231c2a90632997688232192fa72d409bb2b Author: remm <r...@apache.org> AuthorDate: Mon Oct 21 12:09:43 2024 +0200 Improve lock test coverage a bit more Timeout header was likely an ancient style, now it is simpler. Enable strict If header processing by default (for consistency and ease of testing) since it is simple to disable. --- .../apache/catalina/servlets/WebdavServlet.java | 27 ++++++------- .../catalina/servlets/TestWebdavServlet.java | 44 +++++++++++++++++++++- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java b/java/org/apache/catalina/servlets/WebdavServlet.java index 4f1df32636..73fd903bae 100644 --- a/java/org/apache/catalina/servlets/WebdavServlet.java +++ b/java/org/apache/catalina/servlets/WebdavServlet.java @@ -261,7 +261,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen /** * Is the if header processing strict. */ - private boolean strictIfProcessing = false; + private boolean strictIfProcessing = true; // --------------------------------------------------------- Public Methods @@ -1226,23 +1226,14 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen int lockDuration = DEFAULT_TIMEOUT; String lockDurationStr = req.getHeader("Timeout"); if (lockDurationStr != null) { - int commaPos = lockDurationStr.indexOf(','); - // If multiple timeouts, just use the first - if (commaPos != -1) { - lockDurationStr = lockDurationStr.substring(0, commaPos); - } if (lockDurationStr.startsWith("Second-")) { - lockDuration = Integer.parseInt(lockDurationStr.substring(7)); - } else { - if (lockDurationStr.equalsIgnoreCase("infinity")) { - lockDuration = MAX_TIMEOUT; - } else { - try { - lockDuration = Integer.parseInt(lockDurationStr); - } catch (NumberFormatException e) { - lockDuration = MAX_TIMEOUT; - } + try { + lockDuration = Integer.parseInt(lockDurationStr.substring("Second-".length())); + } catch (NumberFormatException e) { + // Ignore } + } else if (lockDurationStr.equals("Infinite")) { + lockDuration = MAX_TIMEOUT; } if (lockDuration == 0) { lockDuration = DEFAULT_TIMEOUT; @@ -1634,6 +1625,10 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen resourceLocks.remove(parentPath); unlocked = true; break; + } else { + // No parent exclusive lock will be found + unlocked = false; + break; } } else { for (String token : parentLock.sharedTokens) { diff --git a/test/org/apache/catalina/servlets/TestWebdavServlet.java b/test/org/apache/catalina/servlets/TestWebdavServlet.java index 71f7977ae4..02f90ee385 100644 --- a/test/org/apache/catalina/servlets/TestWebdavServlet.java +++ b/test/org/apache/catalina/servlets/TestWebdavServlet.java @@ -438,7 +438,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 + - "If: (" + lockToken + ")" + SimpleHttpClient.CRLF + + "If: </myfolder/> (" + lockToken + ")" + SimpleHttpClient.CRLF + "Connection: Close" + SimpleHttpClient.CRLF + SimpleHttpClient.CRLF + CONTENT }); client.connect(); @@ -455,8 +455,18 @@ public class TestWebdavServlet extends TomcatBaseTest { client.processRequest(true); Assert.assertEquals(WebdavStatus.SC_LOCKED, client.getStatusCode()); + client.setRequest(new String[] { "UNLOCK /myfolder/file5.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Lock-Token: <my:locktoken>" + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CONFLICT, client.getStatusCode()); + Assert.assertTrue(client.getResponseBody().contains("<D:href>/myfolder</D:href>")); + // Unlock /myfolder - client.setRequest(new String[] { "UNLOCK /myfolder HTTP/1.1" + SimpleHttpClient.CRLF + + client.setRequest(new String[] { "UNLOCK /myfolder/file5.txt HTTP/1.1" + SimpleHttpClient.CRLF + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + "Lock-Token: " + lockToken + SimpleHttpClient.CRLF + "Connection: Close" + SimpleHttpClient.CRLF + @@ -535,6 +545,7 @@ public class TestWebdavServlet extends TomcatBaseTest { // Lock /myfolder again client.setRequest(new String[] { "LOCK /myfolder HTTP/1.1" + SimpleHttpClient.CRLF + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Timeout: Second-20" + SimpleHttpClient.CRLF + "Content-Length: " + LOCK_BODY.length() + SimpleHttpClient.CRLF + "Connection: Close" + SimpleHttpClient.CRLF + SimpleHttpClient.CRLF + LOCK_BODY }); @@ -599,6 +610,10 @@ public class TestWebdavServlet extends TomcatBaseTest { Assert.assertEquals(WebdavStatus.SC_MULTI_STATUS, client.getStatusCode()); Assert.assertFalse(client.getResponseBody().contains("/myfolder/file4.txt")); Assert.assertTrue(client.getResponseBody().contains("/file7.txt")); + Assert.assertTrue(client.getResponseBody().contains("Second-")); + String timeoutValue = client.getResponseBody().substring(client.getResponseBody().indexOf("Second-")); + timeoutValue = timeoutValue.substring("Second-".length(), timeoutValue.indexOf('<')); + Assert.assertTrue(Integer.valueOf(timeoutValue) <= 20); // Unlock /myfolder again client.setRequest(new String[] { "UNLOCK /myfolder/ HTTP/1.1" + SimpleHttpClient.CRLF + @@ -756,6 +771,7 @@ public class TestWebdavServlet extends TomcatBaseTest { // Lock refresh /myfolder client.setRequest(new String[] { "LOCK /myfolder HTTP/1.1" + SimpleHttpClient.CRLF + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Timeout: Infinite" + SimpleHttpClient.CRLF + "If: (" + lockToken + ")" + SimpleHttpClient.CRLF + "Connection: Close" + SimpleHttpClient.CRLF + SimpleHttpClient.CRLF }); @@ -806,6 +822,10 @@ public class TestWebdavServlet extends TomcatBaseTest { client.processRequest(true); Assert.assertEquals(WebdavStatus.SC_MULTI_STATUS, client.getStatusCode()); Assert.assertTrue(client.getResponseBody().contains("opaquelocktoken:")); + Assert.assertTrue(client.getResponseBody().contains("Second-")); + String timeoutValue = client.getResponseBody().substring(client.getResponseBody().indexOf("Second-")); + timeoutValue = timeoutValue.substring("Second-".length(), timeoutValue.indexOf('<')); + Assert.assertTrue(Integer.valueOf(timeoutValue) > 100000); client.setRequest(new String[] { "PUT /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + @@ -824,8 +844,28 @@ public class TestWebdavServlet extends TomcatBaseTest { SimpleHttpClient.CRLF + CONTENT }); client.connect(); client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_PRECONDITION_FAILED, client.getStatusCode()); + + client.setRequest(new String[] { "PUT /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "If: </myfolder/myfolder2/myfolder4/myfolder5> (" + lockToken + ")" + SimpleHttpClient.CRLF + + "Content-Length: 6" + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + CONTENT }); + client.connect(); + client.processRequest(true); Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + client.setRequest(new String[] { "UNLOCK /myfolder/myfolder3 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Lock-Token: " + lockToken2 + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CONFLICT, client.getStatusCode()); + Assert.assertTrue(client.getResponseBody().contains("<D:lock-token-matches-request-uri/>")); + client.setRequest(new String[] { "UNLOCK /myfolder/myfolder2/myfolder4/myfolder5 HTTP/1.1" + SimpleHttpClient.CRLF + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + "Lock-Token: " + lockToken2 + SimpleHttpClient.CRLF + --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org