Procházet zdrojové kódy

Perform URI normalization based on the result of checking for actually existing files & folders instead of request parameters only. This should fixe MOVE requests on Linux

Sebastian Stenzel před 9 roky
rodič
revize
55bee3d0d5

+ 6 - 4
main/filesystem-api/src/main/java/org/cryptomator/filesystem/Folder.java

@@ -48,10 +48,12 @@ public interface Folder extends Node {
 	/**
 	 * Returns a file by resolving a path relative to this folder.
 	 * 
-	 * @param path A unix-style path, which is always relative to this folder, no matter if it starts with a slash or not
+	 * @param path A unix-style path, which is always relative to this folder, no matter if it starts with a slash or not. Path must not be empty.
 	 * @return File with the given path relative to this folder
+	 * @throws IllegalArgumentException
+	 *             if relativePath is empty
 	 */
-	default File resolveFile(String relativePath) throws UncheckedIOException {
+	default File resolveFile(String relativePath) throws UncheckedIOException, IllegalArgumentException {
 		return PathResolver.resolveFile(this, relativePath);
 	}
 
@@ -68,8 +70,8 @@ public interface Folder extends Node {
 	/**
 	 * Returns a folder by resolving a path relative to this folder.
 	 * 
-	 * @param path A unix-style path, which is always relative to this folder, no matter if it starts with a slash or not
-	 * @return Folder with the given path relative to this folder
+	 * @param path A unix-style path, which is always relative to this folder, no matter if it starts with a slash or not. Path may be empty.
+	 * @return Folder with the given path relative to this folder. Returns <code>this</code> if path is empty.
 	 */
 	default Folder resolveFolder(String relativePath) throws UncheckedIOException {
 		return PathResolver.resolveFolder(this, relativePath);

+ 13 - 1
main/filesystem-api/src/main/java/org/cryptomator/filesystem/PathResolver.java

@@ -45,6 +45,16 @@ final class PathResolver {
 	 * </tr>
 	 * <tr>
 	 * <td>/foo/bar</td>
+	 * <td>/</td>
+	 * <td>/foo/bar</td>
+	 * </tr>
+	 * <tr>
+	 * <td>/foo/bar</td>
+	 * <td></td>
+	 * <td>/foo/bar</td>
+	 * </tr>
+	 * <tr>
+	 * <td>/foo/bar</td>
 	 * <td>../../..</td>
 	 * <td>Exception</td>
 	 * </tr>
@@ -58,7 +68,7 @@ final class PathResolver {
 	public static Folder resolveFolder(Folder dir, String relativePath) {
 		final String[] fragments = StringUtils.split(relativePath, '/');
 		if (ArrayUtils.isEmpty(fragments)) {
-			throw new IllegalArgumentException("Empty relativePath");
+			return dir;
 		}
 		return resolveFolder(dir, Arrays.stream(fragments).iterator());
 	}
@@ -69,6 +79,8 @@ final class PathResolver {
 	 * @param dir The directory from which to resolve the path.
 	 * @param relativePath The path relative to a given directory.
 	 * @return The file with the given path relative to the given dir.
+	 * @throws IllegalArgumentException
+	 *             if relativePath is empty, as this path would resolve to the directory itself, which obviously can't be a file.
 	 */
 	public static File resolveFile(Folder dir, String relativePath) {
 		final String[] fragments = StringUtils.split(relativePath, '/');

+ 7 - 5
main/filesystem-api/src/test/java/org/cryptomator/filesystem/PathResolverTest.java

@@ -27,6 +27,13 @@ public class PathResolverTest {
 		Mockito.doReturn(baz).when(bar).file("baz");
 	}
 
+	@Test
+	public void testResolveSameFolder() {
+		Assert.assertEquals(foo, PathResolver.resolveFolder(foo, ""));
+		Assert.assertEquals(foo, PathResolver.resolveFolder(foo, "/"));
+		Assert.assertEquals(foo, PathResolver.resolveFolder(foo, "///"));
+	}
+
 	@Test
 	public void testResolveChildFolder() {
 		Assert.assertEquals(bar, PathResolver.resolveFolder(root, "foo/bar"));
@@ -50,11 +57,6 @@ public class PathResolverTest {
 		PathResolver.resolveFolder(root, "..");
 	}
 
-	@Test(expected = IllegalArgumentException.class)
-	public void testResolveFolderWithEmptyPath() {
-		PathResolver.resolveFolder(root, "");
-	}
-
 	@Test(expected = IllegalArgumentException.class)
 	public void testResolveFileWithEmptyPath() {
 		PathResolver.resolveFile(root, "");

+ 2 - 9
main/jackrabbit-filesystem-adapter/src/main/java/org/cryptomator/filesystem/jackrabbit/FileSystemResourceLocatorFactory.java

@@ -37,17 +37,10 @@ public class FileSystemResourceLocatorFactory implements DavLocatorFactory {
 	private FileSystemResourceLocator createResourceLocator(String path) {
 		if (StringUtils.isEmpty(path) || "/".equals(path)) {
 			return fs;
-		}
-		final FolderLocator folder = fs.resolveFolder(path);
-		final FileLocator file = fs.resolveFile(path);
-		if (folder.exists()) {
-			return folder;
-		} else if (file.exists()) {
-			return file;
 		} else if (path.endsWith("/")) {
-			return folder;
+			return fs.resolveFolder(path);
 		} else {
-			return file;
+			return fs.resolveFile(path);
 		}
 
 	}

+ 53 - 18
main/jackrabbit-filesystem-adapter/src/main/java/org/cryptomator/webdav/filters/UriNormalizationFilter.java

@@ -11,16 +11,38 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * Depending on the HTTP method a "/" is added to or removed from the end of an URI.
- * For example <code>MKCOL</code> creates a directory (ending on "/"), while <code>PUT</code> creates a file (not ending on "/").
+ * Normalizes all URIs contained in requests depending on the resource type of existing resources.
+ * URIs identifying directories will always end on "/", URIs identifying files will not.
+ * 
+ * If the resource type is unknown, because the resource doesn't exist yet, this filter will determine the resource type based on the HTTP method,
+ * e.g. a MKCOL request will result in a directory..
  */
 public class UriNormalizationFilter implements HttpFilter {
 
+	private static final Logger LOG = LoggerFactory.getLogger(UriNormalizationFilter.class);
 	private static final String[] FILE_METHODS = {"PUT"};
 	private static final String[] DIRECTORY_METHODS = {"MKCOL"};
-	private static final String MOVE = "MOVE";
+
+	@FunctionalInterface
+	public interface ResourceTypeChecker {
+
+		enum ResourceType {
+			FILE, FOLDER, NONEXISTING
+		};
+
+		ResourceType typeOfResource(String resourcePath);
+
+	}
+
+	private final ResourceTypeChecker resourceTypeChecker;
+
+	public UriNormalizationFilter(ResourceTypeChecker resourceTypeChecker) {
+		this.resourceTypeChecker = resourceTypeChecker;
+	}
 
 	@Override
 	public void init(FilterConfig filterConfig) throws ServletException {
@@ -29,14 +51,23 @@ public class UriNormalizationFilter implements HttpFilter {
 
 	@Override
 	public void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
-		if (ArrayUtils.contains(FILE_METHODS, request.getMethod().toUpperCase())) {
+		switch (resourceTypeChecker.typeOfResource(request.getPathInfo())) {
+		case FILE:
 			chain.doFilter(new FileUriRequest(request), response);
-		} else if (ArrayUtils.contains(DIRECTORY_METHODS, request.getMethod().toUpperCase())) {
-			chain.doFilter(new DirectoryUriRequest(request), response);
-		} else if (MOVE.equalsIgnoreCase(request.getMethod())) {
-			chain.doFilter(new CanonicalMoveRequest(request), response);
-		} else {
-			chain.doFilter(request, response);
+			return;
+		case FOLDER:
+			chain.doFilter(new FolderUriRequest(request), response);
+			return;
+		case NONEXISTING:
+		default:
+			if (ArrayUtils.contains(FILE_METHODS, request.getMethod().toUpperCase())) {
+				chain.doFilter(new FileUriRequest(request), response);
+			} else if (ArrayUtils.contains(DIRECTORY_METHODS, request.getMethod().toUpperCase())) {
+				chain.doFilter(new FolderUriRequest(request), response);
+			} else {
+				LOG.warn("Could not determine resource type for URI {}. Leaving request unmodified.", request.getRequestURI());
+				chain.doFilter(request, response);
+			}
 		}
 	}
 
@@ -46,19 +77,21 @@ public class UriNormalizationFilter implements HttpFilter {
 	}
 
 	/**
-	 * Makes the destination header end on "/" if moving a directory and remove additional "/" if moving a file.
+	 * Adjusts headers containing URIs depending on the request URI.
 	 */
-	private static class CanonicalMoveRequest extends HttpServletRequestWrapper {
+	private static class SuffixPreservingRequest extends HttpServletRequestWrapper {
 
-		private static String DESTINATION_HEADER = "Destination";
+		private static final String HEADER_DESTINATION = "Destination";
+		private static final String METHOD_MOVE = "MOVE";
+		private static final String METHOD_COPY = "COPY";
 
-		public CanonicalMoveRequest(HttpServletRequest request) {
+		public SuffixPreservingRequest(HttpServletRequest request) {
 			super(request);
 		}
 
 		@Override
 		public String getHeader(String name) {
-			if (name.equalsIgnoreCase(DESTINATION_HEADER)) {
+			if ((METHOD_MOVE.equalsIgnoreCase(getMethod()) || METHOD_COPY.equalsIgnoreCase(getMethod())) && HEADER_DESTINATION.equalsIgnoreCase(name)) {
 				return sameSuffixAsUri(super.getHeader(name));
 			} else {
 				return super.getHeader(name);
@@ -79,10 +112,11 @@ public class UriNormalizationFilter implements HttpFilter {
 	/**
 	 * HTTP request, whose URI never ends on "/".
 	 */
-	private static class FileUriRequest extends HttpServletRequestWrapper {
+	private static class FileUriRequest extends SuffixPreservingRequest {
 
 		public FileUriRequest(HttpServletRequest request) {
 			super(request);
+			LOG.debug("Treating resource as file: {}", request.getRequestURI());
 		}
 
 		@Override
@@ -95,10 +129,11 @@ public class UriNormalizationFilter implements HttpFilter {
 	/**
 	 * HTTP request, whose URI always ends on "/".
 	 */
-	private static class DirectoryUriRequest extends HttpServletRequestWrapper {
+	private static class FolderUriRequest extends SuffixPreservingRequest {
 
-		public DirectoryUriRequest(HttpServletRequest request) {
+		public FolderUriRequest(HttpServletRequest request) {
 			super(request);
+			LOG.debug("Treating resource as folder: {}", request.getRequestURI());
 		}
 
 		@Override

+ 14 - 1
main/jackrabbit-filesystem-adapter/src/test/java/org/cryptomator/webdav/jackrabbitservlet/FileSystemBasedWebDavServer.java

@@ -21,10 +21,13 @@ import org.cryptomator.webdav.filters.AcceptRangeFilter;
 import org.cryptomator.webdav.filters.LoggingHttpFilter;
 import org.cryptomator.webdav.filters.PutIdleTimeoutFilter;
 import org.cryptomator.webdav.filters.UriNormalizationFilter;
+import org.cryptomator.webdav.filters.UriNormalizationFilter.ResourceTypeChecker;
+import org.cryptomator.webdav.filters.UriNormalizationFilter.ResourceTypeChecker.ResourceType;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.handler.ContextHandlerCollection;
+import org.eclipse.jetty.servlet.FilterHolder;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
@@ -45,6 +48,16 @@ class FileSystemBasedWebDavServer {
 		localConnector.setPort(8080);
 		servletCollection = new ContextHandlerCollection();
 
+		final ResourceTypeChecker resourceTypeChecker = (path) -> {
+			if (fileSystem.resolveFolder(path).exists()) {
+				return ResourceType.FOLDER;
+			} else if (fileSystem.resolveFile(path).exists()) {
+				return ResourceType.FILE;
+			} else {
+				return ResourceType.NONEXISTING;
+			}
+		};
+
 		URI servletContextRootUri;
 		try {
 			servletContextRootUri = new URI("http", null, "localhost", 8080, "/", null, null);
@@ -55,7 +68,7 @@ class FileSystemBasedWebDavServer {
 		final ServletHolder servletHolder = new ServletHolder("FileSystem-WebDAV-Servlet", new WebDavServlet(servletContextRootUri, fileSystem));
 		servletContext.addServlet(servletHolder, "/*");
 		servletContext.addFilter(AcceptRangeFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
-		servletContext.addFilter(UriNormalizationFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
+		servletContext.addFilter(new FilterHolder(new UriNormalizationFilter(resourceTypeChecker)), "/*", EnumSet.of(DispatcherType.REQUEST));
 		servletContext.addFilter(PutIdleTimeoutFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
 		servletContext.addFilter(LoggingHttpFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
 		servletCollection.mapContexts();