This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5d9e6aa71f9a073bb693412a648795e6abc53715 Author: remm <r...@apache.org> AuthorDate: Fri Oct 18 16:48:30 2024 +0200 Rewrite implementation of WebDAV shared locks During my review of RFC 4918, I found the implementation of shared locks was non compliant due to wrong design (one lock with multiple tokens, for example). Exclusive locks are simpler and are mostly unchanged. Collection locks now go into the same map as resource locks (and path traversal is used to discover them) to avoid having to use four maps. Implement new RFC 4918 behavior regarding auth principals and locks. Implement the many new clarifications on unlock and lock discovery. Add a shared locks test case with lots of edge cases. --- .../apache/catalina/servlets/WebdavServlet.java | 529 +++++++++++---------- .../catalina/servlets/TestWebdavServlet.java | 245 +++++++++- webapps/docs/changelog.xml | 21 + 3 files changed, 544 insertions(+), 251 deletions(-) diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java b/java/org/apache/catalina/servlets/WebdavServlet.java index 033a02ff9c..e60b92a560 100644 --- a/java/org/apache/catalina/servlets/WebdavServlet.java +++ b/java/org/apache/catalina/servlets/WebdavServlet.java @@ -29,10 +29,8 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Date; import java.util.Deque; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -215,18 +213,15 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen // ----------------------------------------------------- Instance Variables /** - * Repository of the locks put on single resources. - * <p> - * Key : path <br> - * Value : LockInfo + * Repository of all locks, keyed by path. */ - private final ConcurrentHashMap<String,LockInfo> resourceLocks = new ConcurrentHashMap<>(); + private final ConcurrentHashMap<String, LockInfo> resourceLocks = new ConcurrentHashMap<>(); /** - * List of the inheritable collection locks. + * Map of all shared locks, keyed by lock token. */ - private final CopyOnWriteArrayList<LockInfo> collectionLocks = new CopyOnWriteArrayList<>(); + private final ConcurrentHashMap<String, LockInfo> sharedLocks = new ConcurrentHashMap<>(); /** @@ -282,16 +277,25 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen @Override public void periodicEvent() { // Check expiration of all locks - for (LockInfo currentLock : resourceLocks.values()) { + for (LockInfo currentLock : sharedLocks.values()) { if (currentLock.hasExpired()) { - resourceLocks.remove(currentLock.path); + sharedLocks.remove(currentLock.path); } } - Iterator<LockInfo> collectionLocksIterator = collectionLocks.iterator(); - while (collectionLocksIterator.hasNext()) { - LockInfo currentLock = collectionLocksIterator.next(); - if (currentLock.hasExpired()) { - collectionLocksIterator.remove(); + for (LockInfo currentLock : resourceLocks.values()) { + if (currentLock.isExclusive()) { + if (currentLock.hasExpired()) { + resourceLocks.remove(currentLock.path); + } + } else { + for (String token : currentLock.sharedTokens) { + if (sharedLocks.get(token) == null) { + currentLock.sharedTokens.remove(token); + } + } + if (currentLock.sharedTokens.isEmpty()) { + resourceLocks.remove(currentLock.path); + } } } } @@ -429,8 +433,11 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen if (result.length() == 0) { result.append('/'); } - - return result.toString(); + String resultString = result.toString(); + if (resultString.length() > 1 && resultString.endsWith("/")) { + resultString = resultString.substring(0, resultString.length() - 1); + } + return resultString; } @@ -452,7 +459,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen } else { href += path; } - if (resource.isDirectory() && (!href.endsWith("/"))) { + if (resource != null && resource.isDirectory() && (!href.endsWith("/"))) { href += "/"; } @@ -484,9 +491,6 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen } String path = getRelativePath(req); - if (path.length() > 1 && path.endsWith("/")) { - path = path.substring(0, path.length() - 1); - } // Exclude any resource in the /WEB-INF and /META-INF subdirectories if (isSpecialPath(path)) { @@ -662,9 +666,6 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen protected void doProppatch(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { String path = getRelativePath(req); - if (path.length() > 1 && path.endsWith("/")) { - path = path.substring(0, path.length() - 1); - } WebResource resource = resources.getResource(path); if (!checkIfHeaders(req, resp, resource)) { @@ -1019,16 +1020,13 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen String path = getRelativePath(req); - if (isLocked(path, req)) { - resp.sendError(WebdavStatus.SC_LOCKED); - return; - } WebResource resource = resources.getResource(path); if (!checkIfHeaders(req, resp, resource)) { return; } LockInfo lock = new LockInfo(maxDepth); + lock.principal = req.getRemoteUser(); // Parsing lock request @@ -1214,27 +1212,33 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen 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; + // Check if the resource or a parent is already locked + String parentPath = path; + do { + LockInfo parentLock = resourceLocks.get(parentPath); + if (parentLock != null) { + if (parentLock.hasExpired()) { + resourceLocks.remove(parentPath); + } else if (parentLock.isExclusive() || lock.isExclusive()) { + // A parent collection of this collection is locked + resp.setStatus(WebdavStatus.SC_LOCKED); + return; + } } - if (lock.path.startsWith(currentLock.path + "/") && (currentLock.isExclusive() || lock.isExclusive())) { - // A parent collection of this collection is locked - resp.setStatus(WebdavStatus.SC_LOCKED); - return; + int slash = parentPath.lastIndexOf('/'); + if (slash < 0) { + break; } - } + parentPath = parentPath.substring(0, slash); + } while (true); // Generating lock id String lockTokenStr = req.getServletPath() + "-" + lock.type + "-" + lock.scope + "-" + - req.getUserPrincipal() + "-" + lock.depth + "-" + lock.owner + "-" + lock.tokens + "-" + + req.getUserPrincipal() + "-" + lock.depth + "-" + lock.owner + "-" + lock.token + "-" + lock.expiresAt + "-" + System.currentTimeMillis() + "-" + secret; String lockToken = HexUtils .toHexString(ConcurrentMessageDigest.digestMD5(lockTokenStr.getBytes(StandardCharsets.ISO_8859_1))); + lock.token = lockToken; if (resource.isDirectory() && lock.depth == maxDepth) { @@ -1242,29 +1246,18 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen // Checking if a child resource of this collection is // already locked + // Note: it is likely faster in many cases to go over the full lock list than trying to go over all the + // children (recursively) List<String> lockPaths = new ArrayList<>(); - 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())) { - // A child collection of this collection is locked - lockPaths.add(currentLock.path); - } - } for (LockInfo currentLock : resourceLocks.values()) { if (currentLock.hasExpired()) { resourceLocks.remove(currentLock.path); continue; } - if (currentLock.path.startsWith(lock.path + "/") && - (currentLock.isExclusive() || lock.isExclusive())) { + if ((currentLock.isExclusive() || lock.isExclusive()) && + currentLock.path.startsWith(lock.path + "/")) { // A child resource of this collection is locked - lockPaths.add(currentLock.path); + lockPaths.add(currentLock.lockroot); } } @@ -1280,6 +1273,14 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen generatedXML.writeElement("D", DEFAULT_NAMESPACE, "multistatus", XMLWriter.OPENING); + generatedXML.writeElement("D", "response", XMLWriter.OPENING); + generatedXML.writeElement("D", "href", XMLWriter.OPENING); + generatedXML.writeText(getEncodedPath(path, resource, req)); + generatedXML.writeElement("D", "href", XMLWriter.CLOSING); + generatedXML.writeElement("D", "status", XMLWriter.OPENING); + generatedXML.writeText("HTTP/1.1 " + WebdavStatus.SC_FAILED_DEPENDENCY + " "); + generatedXML.writeElement("D", "status", XMLWriter.CLOSING); + generatedXML.writeElement("D", "response", XMLWriter.CLOSING); for (String lockPath : lockPaths) { generatedXML.writeElement("D", "response", XMLWriter.OPENING); generatedXML.writeElement("D", "href", XMLWriter.OPENING); @@ -1300,73 +1301,38 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen return; } - boolean addLock = true; - - // Checking if there is already a shared lock on this path - for (LockInfo currentLock : collectionLocks) { - if (currentLock.path.equals(lock.path)) { - - if (currentLock.isExclusive()) { - resp.sendError(WebdavStatus.SC_LOCKED); - return; - } else { - if (lock.isExclusive()) { - resp.sendError(WebdavStatus.SC_LOCKED); - return; - } - } - - currentLock.tokens.add(lockToken); - lock = currentLock; - addLock = false; - } - } - - if (addLock) { - lock.tokens.add(lockToken); - collectionLocks.add(lock); - // Add the Lock-Token header as by RFC 2518 8.10.1 - // - only do this for newly created locks - resp.addHeader("Lock-Token", "<opaquelocktoken:" + lockToken + ">"); - } - } else { // Locking a single resource - // Retrieving an already existing lock on that resource - LockInfo presentLock = resourceLocks.get(lock.path); - if (presentLock != null) { - - if ((presentLock.isExclusive()) || (lock.isExclusive())) { - // If either lock is exclusive, the lock can't be - // granted - resp.sendError(WebdavStatus.SC_PRECONDITION_FAILED); + // Checking if a resource exists at this path + if (!resource.exists()) { + // RFC 4918 removes lock null, instead an empty file is created + if (!resources.write(path, new ByteArrayInputStream(new byte[0]), false)) { + resp.sendError(WebdavStatus.SC_CONFLICT); return; - } else { - presentLock.tokens.add(lockToken); - lock = presentLock; - } - - } else { - - // Checking if a resource exists at this path - if (!resource.exists()) { - // RFC 4918 removes lock null, instead an empty file is created - if (!resources.write(path, new ByteArrayInputStream(new byte[0]), false)) { - resp.sendError(WebdavStatus.SC_CONFLICT); - return; - } } + } - lock.tokens.add(lockToken); - resourceLocks.put(lock.path, lock); + } - // Add the Lock-Token header as by RFC 2518 8.10.1 - // - only do this for newly created locks - resp.addHeader("Lock-Token", "<opaquelocktoken:" + lockToken + ">"); + lock.lockroot = getEncodedPath(lock.path, resource, req); + if (lock.isExclusive()) { + resourceLocks.put(path, lock); + } else { + // Checking if there is already a shared lock on this path + LockInfo sharedLock = resourceLocks.get(path); + if (sharedLock == null) { + resourceLocks.put(path, lock); + sharedLock = lock; } + sharedLock.sharedTokens.add(lockToken); + sharedLocks.put(lockToken, lock); } + + // Add the Lock-Token header as by RFC 2518 8.10.1 + resp.addHeader("Lock-Token", "<opaquelocktoken:" + lockToken + ">"); + } if (lockRequestType == LOCK_REFRESH) { @@ -1383,22 +1349,19 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen LockInfo toRenew = resourceLocks.get(path); if (toRenew != null) { - // At least one of the tokens of the locks must have been given - for (String token : toRenew.tokens) { - if (ifHeader.contains(token)) { + if (toRenew.isExclusive()) { + if (ifHeader.contains(toRenew.token)) { toRenew.expiresAt = lock.expiresAt; lock = toRenew; } - } - } - - // Checking inheritable collection locks - for (LockInfo collecionLock : collectionLocks) { - if (path.equals(collecionLock.path)) { - for (String token : collecionLock.tokens) { + } else { + for (String token : toRenew.sharedTokens) { if (ifHeader.contains(token)) { - collecionLock.expiresAt = lock.expiresAt; - lock = collecionLock; + toRenew = sharedLocks.get(token); + if (toRenew != null) { + toRenew.expiresAt = lock.expiresAt; + lock = toRenew; + } } } } @@ -1442,6 +1405,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen } String path = getRelativePath(req); + WebResource resource = resources.getResource(path); if (!checkIfHeaders(req, resp, resource)) { return; @@ -1452,50 +1416,55 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen lockTokenHeader = ""; } - // Checking resource locks - - LockInfo lock = resourceLocks.get(path); - if (lock != null) { - - // At least one of the tokens of the locks must have been given - Iterator<String> tokenList = lock.tokens.iterator(); - while (tokenList.hasNext()) { - String token = tokenList.next(); - if (lockTokenHeader.contains(token)) { - tokenList.remove(); + boolean unlocked = false; + String parentPath = path; + do { + LockInfo parentLock = resourceLocks.get(parentPath); + if (parentLock != null) { + if (parentLock.hasExpired()) { + resourceLocks.remove(parentPath); + } else { + if ((parentPath != path && parentLock.depth > 0) || parentPath == path) { + if (parentLock.isExclusive()) { + if (lockTokenHeader.contains(parentLock.token) + && (parentLock.principal == null || parentLock.principal.equals(req.getRemoteUser()))) { + resourceLocks.remove(parentPath); + unlocked = true; + break; + } + } else { + for (String token : parentLock.sharedTokens) { + if (lockTokenHeader.contains(token)) { + LockInfo lock = sharedLocks.get(token); + if (lock == null || lock.principal == null || lock.principal.equals(req.getRemoteUser())) { + parentLock.sharedTokens.remove(token); + if (parentLock.sharedTokens.isEmpty()) { + resourceLocks.remove(parentPath); + } + sharedLocks.remove(token); + unlocked = true; + break; + } + } + } + } + } } } - - if (lock.tokens.isEmpty()) { - resourceLocks.remove(path); + int slash = parentPath.lastIndexOf('/'); + if (slash < 0) { + break; } + parentPath = parentPath.substring(0, slash); + } while (true); + if (unlocked) { + resp.setStatus(WebdavStatus.SC_NO_CONTENT); + } else { + sendReport(req, resp, parentPath, WebdavStatus.SC_CONFLICT, "lock-token-matches-request-uri"); } - - // Checking inheritable collection locks - Iterator<LockInfo> collectionLocksList = collectionLocks.iterator(); - while (collectionLocksList.hasNext()) { - lock = collectionLocksList.next(); - if (path.equals(lock.path)) { - Iterator<String> tokenList = lock.tokens.iterator(); - while (tokenList.hasNext()) { - String token = tokenList.next(); - if (lockTokenHeader.contains(token)) { - tokenList.remove(); - break; - } - } - if (lock.tokens.isEmpty()) { - collectionLocks.remove(lock); - break; - } - } - } - - resp.setStatus(WebdavStatus.SC_NO_CONTENT); } - // -------------------------------------------------------- Private Methods /** @@ -1525,7 +1494,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen lockTokenHeader = ""; } - return isLocked(path, ifHeader + lockTokenHeader); + return isLocked(path, req.getRemoteUser(), ifHeader + lockTokenHeader); } @@ -1533,53 +1502,52 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen * Check to see if a resource is currently write locked. * * @param path Path of the resource + * @param principal The authenticated principal name * @param ifHeader "If" HTTP header which was included in the request * * @return <code>true</code> if the resource is locked (and no appropriate lock token has been found for at least * one of the non-shared locks which are present on the resource). */ - private boolean isLocked(String path, String ifHeader) { - - // Checking resource locks - - LockInfo lock = resourceLocks.get(path); - if ((lock != null) && (lock.hasExpired())) { - resourceLocks.remove(path); - } else if (lock != null) { - - // At least one of the tokens of the locks must have been given - - boolean tokenMatch = false; - for (String token : lock.tokens) { - if (ifHeader.contains(token)) { - tokenMatch = true; - break; - } - } - if (!tokenMatch) { - return true; - } - } + private boolean isLocked(String path, String principal, String ifHeader) { - // Checking inheritable collection locks - Iterator<LockInfo> collectionLockList = collectionLocks.iterator(); - while (collectionLockList.hasNext()) { - lock = collectionLockList.next(); - if (lock.hasExpired()) { - collectionLockList.remove(); - } else if (path.startsWith(lock.path)) { - boolean tokenMatch = false; - for (String token : lock.tokens) { - if (ifHeader.contains(token)) { - tokenMatch = true; - break; + // Check if the resource or a parent is already locked + String parentPath = path; + do { + LockInfo parentLock = resourceLocks.get(parentPath); + if (parentLock != null) { + if (parentLock.hasExpired()) { + resourceLocks.remove(parentPath); + } else { + if ((parentPath != path && parentLock.depth > 0) || parentPath == path) { + if (parentLock.isExclusive()) { + if (ifHeader.contains(parentLock.token) + && (parentLock.principal == null || parentLock.principal.equals(principal))) { + return false; + } + } else { + for (String token : parentLock.sharedTokens) { + if (ifHeader.contains(token)) { + if (principal == null) { + return false; + } else { + LockInfo lock = sharedLocks.get(token); + if (lock == null || lock.principal == null || lock.principal.equals(principal)) { + return false; + } + } + } + } + } } - } - if (!tokenMatch) { return true; } } - } + int slash = parentPath.lastIndexOf('/'); + if (slash < 0) { + break; + } + parentPath = parentPath.substring(0, slash); + } while (true); return false; } @@ -1653,6 +1621,10 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen } } + if (destinationPath.length() > 1 && destinationPath.endsWith("/")) { + destinationPath = destinationPath.substring(0, destinationPath.length() - 1); + } + // Cross-context operations aren't supported String reqContextPath = getPathPrefix(req); if (!destinationPath.startsWith(reqContextPath + "/")) { @@ -1891,7 +1863,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen sendNotAllowed(req, resp); return false; } - resourceLocks.remove(path); + unlockResource(path, null); } else { Map<String,Integer> errorList = new LinkedHashMap<>(); @@ -1914,15 +1886,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen errorList.put(path, Integer.valueOf(WebdavStatus.SC_METHOD_NOT_ALLOWED)); } } else { - Iterator<LockInfo> collectionLocksList = collectionLocks.iterator(); - while (collectionLocksList.hasNext()) { - LockInfo lock = collectionLocksList.next(); - if (path.equals(lock.path)) { - collectionLocks.remove(lock); - break; - } - } - resourceLocks.remove(path); + unlockResource(path, null); } if (!errorList.isEmpty()) { @@ -1936,6 +1900,27 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen return true; } + private void unlockResource(String path, String lockToken) { + LockInfo lock = resourceLocks.get(path); + if (lock != null) { + if (lock.isExclusive()) { + if (lockToken == null || lockToken.contains(lock.token)) { + resourceLocks.remove(path); + } + } else { + for (String token : lock.sharedTokens) { + if (lockToken == null || lockToken.contains(token)) { + lock.sharedTokens.remove(token); + if (lock.sharedTokens.isEmpty()) { + resourceLocks.remove(lock.path); + } + sharedLocks.remove(token); + break; + } + } + } + } + } /** * Deletes a collection. @@ -1975,7 +1960,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen } childName += entry; - if (isLocked(childName, lockHeader)) { + if (isLocked(childName, req.getRemoteUser(), lockHeader)) { errorList.put(childName, Integer.valueOf(WebdavStatus.SC_LOCKED)); @@ -2002,17 +1987,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen errorList.put(childName, Integer.valueOf(WebdavStatus.SC_METHOD_NOT_ALLOWED)); } } else { - if (childResource.isDirectory()) { - Iterator<LockInfo> collectionLocksList = collectionLocks.iterator(); - while (collectionLocksList.hasNext()) { - LockInfo lock = collectionLocksList.next(); - if (childName.equals(lock.path)) { - collectionLocks.remove(lock); - break; - } - } - } - resourceLocks.remove(childName); + unlockResource(childName, null); } } } @@ -2045,7 +2020,7 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen generatedXML.writeElement("D", "response", XMLWriter.OPENING); generatedXML.writeElement("D", "href", XMLWriter.OPENING); - generatedXML.writeText(getPathPrefix(req) + errorPath); + generatedXML.writeText(getEncodedPath(errorPath, null, req)); generatedXML.writeElement("D", "href", XMLWriter.CLOSING); generatedXML.writeElement("D", "status", XMLWriter.OPENING); @@ -2063,6 +2038,33 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen } + private void sendReport(HttpServletRequest req, HttpServletResponse resp, String errorPath, int errorCode, String error) + throws IOException { + resp.setStatus(errorCode); + + XMLWriter generatedXML = new XMLWriter(); + generatedXML.writeXMLHeader(); + + generatedXML.writeElement("D", DEFAULT_NAMESPACE, "error", XMLWriter.OPENING); + + if (errorPath != null && errorPath.length() > 0) { + generatedXML.writeElement("D", error, XMLWriter.OPENING); + generatedXML.writeElement("D", "href", XMLWriter.OPENING); + generatedXML.writeText(getEncodedPath(errorPath, null, req)); + generatedXML.writeElement("D", "href", XMLWriter.CLOSING); + generatedXML.writeElement("D", error, XMLWriter.CLOSING); + } else { + generatedXML.writeElement("D", error, XMLWriter.NO_CONTENT); + } + + generatedXML.writeElement("D", "error", XMLWriter.CLOSING); + + Writer writer = resp.getWriter(); + writer.write(generatedXML.toString()); + writer.close(); + } + + private void propfindResource(XMLWriter generatedXML, String rewrittenUrl, String path, int propFindType, List<Node> properties, boolean isFile, long created, long lastModified, long contentLength, String contentType, String eTag) { @@ -2291,25 +2293,47 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen */ private boolean generateLockDiscovery(String path, XMLWriter generatedXML) { - LockInfo resourceLock = resourceLocks.get(path); - boolean wroteStart = false; - if (resourceLock != null) { - wroteStart = true; - generatedXML.writeElement("D", "lockdiscovery", XMLWriter.OPENING); - resourceLock.toXML(generatedXML); - } - - for (LockInfo currentLock : collectionLocks) { - if (path.startsWith(currentLock.path)) { - if (!wroteStart) { - wroteStart = true; - generatedXML.writeElement("D", "lockdiscovery", XMLWriter.OPENING); + String parentPath = path; + do { + LockInfo parentLock = resourceLocks.get(parentPath); + if (parentLock != null) { + if (parentLock.hasExpired()) { + resourceLocks.remove(parentPath); + } else { + if ((parentPath != path && parentLock.depth > 0) || parentPath == path) { + if (parentLock.isExclusive()) { + if (!wroteStart) { + wroteStart = true; + generatedXML.writeElement("D", "lockdiscovery", XMLWriter.OPENING); + } + parentLock.toXML(generatedXML); + } else { + for (String lockToken : parentLock.sharedTokens) { + parentLock = sharedLocks.get(lockToken); + if (parentLock != null) { + if (parentLock.hasExpired()) { + sharedLocks.remove(lockToken); + } else { + if (!wroteStart) { + wroteStart = true; + generatedXML.writeElement("D", "lockdiscovery", XMLWriter.OPENING); + } + parentLock.toXML(generatedXML); + } + } + } + } + } } - currentLock.toXML(generatedXML); } - } + int slash = parentPath.lastIndexOf('/'); + if (slash < 0) { + break; + } + parentPath = parentPath.substring(0, slash); + } while (true); if (wroteStart) { generatedXML.writeElement("D", "lockdiscovery", XMLWriter.CLOSING); @@ -2393,12 +2417,15 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen private final int maxDepth; String path = "/"; + String lockroot = "/"; String type = "write"; String scope = "exclusive"; int depth = 0; String owner = ""; - List<String> tokens = Collections.synchronizedList(new ArrayList<>()); + String token = ""; + List<String> sharedTokens = new CopyOnWriteArrayList<>(); long expiresAt = 0; + String principal = null; // ----------------------------------------------------- Public Methods @@ -2416,10 +2443,8 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen result.append(owner); result.append("\nExpiration:"); result.append(FastHttpDateFormat.formatDate(expiresAt)); - for (String token : tokens) { - result.append("\nToken:"); - result.append(token); - } + result.append("\nToken:"); + result.append(token); result.append("\n"); return result.toString(); } @@ -2475,12 +2500,16 @@ public class WebdavServlet extends DefaultServlet implements PeriodicEventListen generatedXML.writeText("Second-" + timeout); generatedXML.writeElement("D", "timeout", XMLWriter.CLOSING); + generatedXML.writeElement("D", "lockroot", XMLWriter.OPENING); + generatedXML.writeElement("D", "href", XMLWriter.OPENING); + generatedXML.writeText(lockroot); + generatedXML.writeElement("D", "href", XMLWriter.CLOSING); + generatedXML.writeElement("D", "lockroot", XMLWriter.CLOSING); + generatedXML.writeElement("D", "locktoken", XMLWriter.OPENING); - for (String token : tokens) { - generatedXML.writeElement("D", "href", XMLWriter.OPENING); - generatedXML.writeText("opaquelocktoken:" + token); - generatedXML.writeElement("D", "href", XMLWriter.CLOSING); - } + generatedXML.writeElement("D", "href", XMLWriter.OPENING); + generatedXML.writeText("opaquelocktoken:" + token); + generatedXML.writeElement("D", "href", XMLWriter.CLOSING); generatedXML.writeElement("D", "locktoken", XMLWriter.CLOSING); generatedXML.writeElement("D", "activelock", XMLWriter.CLOSING); diff --git a/test/org/apache/catalina/servlets/TestWebdavServlet.java b/test/org/apache/catalina/servlets/TestWebdavServlet.java index 7cf7418c69..2c4b217290 100644 --- a/test/org/apache/catalina/servlets/TestWebdavServlet.java +++ b/test/org/apache/catalina/servlets/TestWebdavServlet.java @@ -198,6 +198,14 @@ public class TestWebdavServlet extends TomcatBaseTest { " <D:owner>someone</D:owner>\n" + "</D:lockinfo>"; + private static final String LOCK_SHARED_BODY = + "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" + + "<D:lockinfo xmlns:D='DAV:'>\n" + + " <D:lockscope><D:shared/></D:lockscope>\n" + + " <D:locktype><D:write/></D:locktype>\n" + + " <D:owner>someone</D:owner>\n" + + "</D:lockinfo>"; + private static final String PROPFIND_PROP = "<?xml version=\"1.0\" encoding=\"utf-8\" ?> \n" + "<D:propfind xmlns:D=\"DAV:\">\n" + @@ -553,7 +561,7 @@ public class TestWebdavServlet extends TomcatBaseTest { Assert.assertTrue(client.getResponseBody().contains("/file7.txt")); // Unlock /myfolder again - client.setRequest(new String[] { "UNLOCK /myfolder HTTP/1.1" + SimpleHttpClient.CRLF + + 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 + @@ -582,6 +590,241 @@ public class TestWebdavServlet extends TomcatBaseTest { } + @Test + public void testSharedLocks() throws Exception { + Tomcat tomcat = getTomcatInstance(); + + // Create a temp webapp that can be safely written to + File tempWebapp = new File(getTemporaryDirectory(), "webdav-lock"); + Assert.assertTrue(tempWebapp.mkdirs()); + Context ctxt = tomcat.addContext("", tempWebapp.getAbsolutePath()); + Wrapper webdavServlet = Tomcat.addServlet(ctxt, "webdav", new WebdavServlet()); + webdavServlet.addInitParameter("listings", "true"); + webdavServlet.addInitParameter("secret", "foo"); + webdavServlet.addInitParameter("readonly", "false"); + ctxt.addServletMappingDecoded("/*", "webdav"); + tomcat.start(); + + Client client = new Client(); + client.setPort(getPort()); + + // Create a few folders and files + client.setRequest(new String[] { "MKCOL /myfolder HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + + client.setRequest(new String[] { "MKCOL /myfolder/myfolder2/ HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + + client.setRequest(new String[] { "MKCOL /myfolder/myfolder3 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + + client.setRequest(new String[] { "MKCOL /myfolder/myfolder2/myfolder4 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + + client.setRequest(new String[] { "MKCOL /myfolder/myfolder2/myfolder4/myfolder5 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_CREATED, client.getStatusCode()); + + client.setRequest(new String[] { "PUT /file1.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + 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[] { "PUT /myfolder/myfolder3/file2.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + 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[] { "PUT /myfolder/myfolder2/myfolder4/file3.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + 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()); + + // Lock /myfolder + client.setRequest(new String[] { "LOCK /myfolder HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Content-Length: " + LOCK_SHARED_BODY.length() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + LOCK_SHARED_BODY }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_OK, client.getStatusCode()); + Assert.assertTrue(client.getResponseBody().contains("opaquelocktoken:")); + String lockToken = null; + for (String header : client.getResponseHeaders()) { + if (header.startsWith("Lock-Token: ")) { + lockToken = header.substring("Lock-Token: ".length()); + } + } + Assert.assertNotNull(lockToken); + + client.setRequest(new String[] { "LOCK /myfolder/myfolder2/myfolder4/myfolder5 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "If: " + lockToken + SimpleHttpClient.CRLF + + "Content-Length: " + LOCK_BODY.length() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + LOCK_BODY }); + client.connect(); + client.processRequest(true); + // This should conflict, submitting a token does not help + Assert.assertEquals(WebdavStatus.SC_LOCKED, client.getStatusCode()); + + client.setRequest(new String[] { "LOCK /myfolder/myfolder2/myfolder4 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Content-Length: " + LOCK_SHARED_BODY.length() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + LOCK_SHARED_BODY }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_OK, client.getStatusCode()); + Assert.assertTrue(client.getResponseBody().contains("opaquelocktoken:")); + String lockToken2 = null; + for (String header : client.getResponseHeaders()) { + if (header.startsWith("Lock-Token: ")) { + lockToken2 = header.substring("Lock-Token: ".length()); + } + } + Assert.assertNotNull(lockToken2); + + // Take a second different lock on the same collection + client.setRequest(new String[] { "LOCK /myfolder/myfolder2/myfolder4 HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Content-Length: " + LOCK_SHARED_BODY.length() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + LOCK_SHARED_BODY }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(HttpServletResponse.SC_OK, client.getStatusCode()); + Assert.assertTrue(client.getResponseBody().contains("opaquelocktoken:")); + String lockToken3 = null; + for (String header : client.getResponseHeaders()) { + if (header.startsWith("Lock-Token: ")) { + lockToken3 = header.substring("Lock-Token: ".length()); + } + } + Assert.assertNotNull(lockToken3); + + client.setRequest(new String[] { "PROPFIND / HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(WebdavStatus.SC_MULTI_STATUS, client.getStatusCode()); + Assert.assertTrue(client.getResponseBody().contains("opaquelocktoken:")); + + client.setRequest(new String[] { "PUT /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Content-Length: 6" + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + CONTENT }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(WebdavStatus.SC_LOCKED, client.getStatusCode()); + + client.setRequest(new String[] { "PUT /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "If: " + lockToken2 + 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/myfolder2/myfolder4/myfolder5 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_NO_CONTENT, client.getStatusCode()); + + client.setRequest(new String[] { "UNLOCK /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Lock-Token: " + lockToken3 + 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[] { "PUT /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Content-Length: 6" + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + CONTENT }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(WebdavStatus.SC_LOCKED, client.getStatusCode()); + + client.setRequest(new String[] { "UNLOCK /myfolder/myfolder2/myfolder4/myfolder5/file4.txt 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[] { "PUT /myfolder/myfolder2/myfolder4/myfolder5/file4.txt HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Content-Length: 12" + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + CONTENT + CONTENT}); + client.connect(); + client.processRequest(true); + Assert.assertEquals(WebdavStatus.SC_NO_CONTENT, client.getStatusCode()); + + client.setRequest(new String[] { "PROPFIND / HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF + + "Connection: Close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }); + client.connect(); + client.processRequest(true); + Assert.assertEquals(WebdavStatus.SC_MULTI_STATUS, client.getStatusCode()); + // Verify all the shared locks are cleared + Assert.assertFalse(client.getResponseBody().contains("opaquelocktoken:")); + + } + private static final class Client extends SimpleHttpClient { @Override diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 3af636641c..b325f810b6 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -107,6 +107,27 @@ <section name="Tomcat 9.0.97 (remm)" rtext="in development"> <subsection name="Catalina"> <changelog> +<<<<<<< HEAD +======= + <add> + Add support for the new Servlet API method + <code>HttpServletResponse.sendEarlyHints()</code>. (markt) + </add> + <update> + Remove default value (was <code>catalina</code>) for the + <code>secret</code> init parameter of the WebDAV Servlet. (remm) + </update> + <update> + Remove WebDAV lock null support in accordance with RFC 4918 section 7.3 + and annex D. Instead a lock on a non existing resource will create an + empty file locked with a regular lock. (remm) + </update> + <update> + Rewrite implementation of WebDAV shared locks to comply with RFC 4918. + (remm) + </update> + <!-- Entries for backport and removal before 12.0.0-M1 below this line --> +>>>>>>> ac76965a76 (Rewrite implementation of WebDAV shared locks) <update> <bug>69374</bug>: Properly separate between table header and body in <code>DefaultServlet</code>'s listing. (michaelo) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org