- Changed OS X PUT request filter from timeout-controlled to headerfield-controlled

- added tests
This commit is contained in:
Sebastian Stenzel
2016-01-13 18:11:22 +01:00
parent 9c844e626a
commit bf0988bb20
6 changed files with 437 additions and 131 deletions

View File

@@ -49,7 +49,11 @@
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
</dependency>
<!-- Test -->
<dependency>
<groupId>org.cryptomator</groupId>

View File

@@ -0,0 +1,146 @@
package org.cryptomator.webdav.filters;
import java.io.IOException;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ReadListener;
import javax.servlet.ServletException;
import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.io.input.BoundedInputStream;
/**
* If a PUT request with chunked transfer encoding and a X-Expected-Entity-Length header field is sent,
* the input stream will return EOF after the number of bytes stated in this header has been read.
*
* This filter ensures compatibility of the Mac OS X WebDAV client, as Macs don't terminate chunked transfers normally (by sending a 0-byte-chunk).
*/
public class MacChunkedPutCompatibilityFilter implements HttpFilter {
private static final String METHOD_PUT = "PUT";
private static final String HEADER_TRANSFER_ENCODING = "Transfer-Encoding";
private static final String HEADER_X_EXPECTED_ENTITIY_LENGTH = "X-Expected-Entity-Length";
private static final String HEADER_TRANSFER_ENCODING_CHUNKED = "chunked";
@Override
public void init(FilterConfig filterConfig) throws ServletException {
// no-op
}
@Override
public void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
final String expectedEntitiyLengthHeader = request.getHeader(HEADER_X_EXPECTED_ENTITIY_LENGTH);
if (METHOD_PUT.equalsIgnoreCase(request.getMethod()) //
&& HEADER_TRANSFER_ENCODING_CHUNKED.equalsIgnoreCase(request.getHeader(HEADER_TRANSFER_ENCODING)) //
&& expectedEntitiyLengthHeader != null) {
long expectedEntitiyLength;
try {
expectedEntitiyLength = Long.valueOf(expectedEntitiyLengthHeader);
} catch (NumberFormatException e) {
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Invalid X-Expected-Entity-Length");
return;
}
chain.doFilter(new PutRequestWithBoundedInputStream(request, expectedEntitiyLength), response);
} else {
chain.doFilter(request, response);
}
}
@Override
public void destroy() {
// no-op
}
private static class PutRequestWithBoundedInputStream extends HttpServletRequestWrapper {
private final long inputStreamLimit;
public PutRequestWithBoundedInputStream(HttpServletRequest request, long inputStreamLimit) {
super(request);
this.inputStreamLimit = inputStreamLimit;
}
@Override
public ServletInputStream getInputStream() throws IOException {
return new BoundedServletInputStream(super.getInputStream(), inputStreamLimit);
}
}
/**
* Like {@link BoundedInputStream}, but as a ServletInputStream.
*/
private static class BoundedServletInputStream extends ServletInputStream {
private final BoundedInputStream boundedIn;
private final ServletInputStream servletIn;
private boolean reachedEof = false;
private ReadListener readListener;
public BoundedServletInputStream(ServletInputStream delegate, long limit) {
this.boundedIn = new BoundedInputStream(delegate, limit);
this.servletIn = delegate;
}
private void reachedEof() throws IOException {
reachedEof = true;
if (readListener != null) {
readListener.onAllDataRead();
}
}
/* BoundedInputStream */
@Override
public long skip(long n) throws IOException {
return boundedIn.skip(n);
}
@Override
public int available() throws IOException {
return boundedIn.available();
}
@Override
public int read(byte[] b, int off, int len) throws IOException {
int read = boundedIn.read(b, off, len);
if (read == -1) {
reachedEof();
}
return read;
}
@Override
public int read() throws IOException {
int aByte = boundedIn.read();
if (aByte == -1) {
reachedEof();
}
return aByte;
}
/* ServletInputStream */
@Override
public boolean isFinished() {
return reachedEof || servletIn.isFinished();
}
@Override
public boolean isReady() {
return !reachedEof && servletIn.isReady();
}
@Override
public void setReadListener(ReadListener readListener) {
servletIn.setReadListener(readListener);
this.readListener = readListener;
}
}
}

View File

@@ -1,128 +0,0 @@
package org.cryptomator.webdav.filters;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ReadListener;
import javax.servlet.ServletException;
import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
/**
* Wrapps the {@link ServletInputStream} into a timeout-aware stream, that returns EOF after a certain timeout.
* Wrapping is done only for chunked PUT requests, as some WebDAV clients are too stupid to send a EOF-chunk (0-byte-chunk).
*/
public class PutIdleTimeoutFilter implements HttpFilter {
private static final long TIMEOUT = 100;
private static final TimeUnit TIMEOUT_UNIT = TimeUnit.MILLISECONDS;
private static final String METHOD_PUT = "PUT";
private static final String HEADER_TRANSFER_ENCODING = "Transfer-Encoding";
private static final String HEADER_TRANSFER_ENCODING_CHUNKED = "chunked";
private final ExecutorService executor = Executors.newSingleThreadExecutor();
@Override
public void init(FilterConfig filterConfig) throws ServletException {
// no-op
}
@Override
public void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
if (METHOD_PUT.equalsIgnoreCase(request.getMethod()) && HEADER_TRANSFER_ENCODING_CHUNKED.equalsIgnoreCase(request.getHeader(HEADER_TRANSFER_ENCODING))) {
chain.doFilter(new PutRequestWithIdleTimeout(request), response);
} else {
chain.doFilter(request, response);
}
}
@Override
public void destroy() {
executor.shutdownNow();
}
private class PutRequestWithIdleTimeout extends HttpServletRequestWrapper {
public PutRequestWithIdleTimeout(HttpServletRequest request) {
super(request);
}
@Override
public ServletInputStream getInputStream() throws IOException {
return new IdleTimeoutServletInputStream(super.getInputStream());
}
}
private class IdleTimeoutServletInputStream extends ServletInputStream {
private final ServletInputStream delegate;
private boolean timedOut = false;
public IdleTimeoutServletInputStream(ServletInputStream delegate) {
this.delegate = delegate;
}
@Override
public boolean isFinished() {
return timedOut || delegate.isFinished();
}
@Override
public boolean isReady() {
return !timedOut && delegate.isReady();
}
@Override
public void setReadListener(ReadListener readListener) {
delegate.setReadListener(readListener);
}
@Override
public int read(byte[] b, int off, int len) throws IOException {
try {
Future<Integer> readTask = executor.submit(() -> {
return delegate.read(b, off, len);
});
return readTask.get(TIMEOUT, TIMEOUT_UNIT);
} catch (InterruptedException e) {
throw new InterruptedIOException();
} catch (ExecutionException e) {
throw new IOException("Exception during read", e);
} catch (TimeoutException e) {
timedOut = true;
return -1;
}
}
@Override
public int read() throws IOException {
try {
Future<Integer> readTask = executor.submit(() -> {
return delegate.read();
});
return readTask.get(TIMEOUT, TIMEOUT_UNIT);
} catch (InterruptedException e) {
throw new InterruptedIOException();
} catch (ExecutionException e) {
throw new IOException("Exception during read", e);
} catch (TimeoutException e) {
// throw new InterruptedByTimeoutException();
timedOut = true;
return -1;
}
}
}
}

View File

@@ -0,0 +1,122 @@
package org.cryptomator.webdav.filters;
import java.io.IOException;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.ServletInputStream;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
public class MacChunkedPutCompatibilityFilterTest {
private MacChunkedPutCompatibilityFilter filter;
private FilterChain chain;
private HttpServletRequest request;
private HttpServletResponse response;
@Before
public void setup() {
filter = new MacChunkedPutCompatibilityFilter();
chain = Mockito.mock(FilterChain.class);
request = Mockito.mock(HttpServletRequest.class);
response = Mockito.mock(HttpServletResponse.class);
}
@Test
public void testUnfilteredGetRequest() throws IOException, ServletException {
Mockito.when(request.getMethod()).thenReturn("GET");
filter.doFilter(request, response, chain);
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertSame(request, wrappedReq.getValue());
}
@Test
public void testUnfilteredPutRequest1() throws IOException, ServletException {
Mockito.when(request.getMethod()).thenReturn("PUT");
Mockito.when(request.getHeader("Transfer-Encoding")).thenReturn(null);
filter.doFilter(request, response, chain);
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertSame(request, wrappedReq.getValue());
}
@Test
public void testUnfilteredPutRequest2() throws IOException, ServletException {
Mockito.when(request.getMethod()).thenReturn("PUT");
Mockito.when(request.getHeader("Transfer-Encoding")).thenReturn("chunked");
Mockito.when(request.getHeader("X-Expected-Entity-Length")).thenReturn(null);
filter.doFilter(request, response, chain);
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertSame(request, wrappedReq.getValue());
}
@Test
public void testMalformedXExpectedEntityLengthHeader() throws IOException, ServletException {
Mockito.when(request.getMethod()).thenReturn("PUT");
Mockito.when(request.getHeader("Transfer-Encoding")).thenReturn("chunked");
Mockito.when(request.getHeader("X-Expected-Entity-Length")).thenReturn("NaN");
filter.doFilter(request, response, chain);
Mockito.verify(response).sendError(Mockito.eq(HttpServletResponse.SC_BAD_REQUEST), Mockito.anyString());
Mockito.verifyNoMoreInteractions(chain);
}
/* actual input stream testing */
@Test
public void testBoundedInputStream() throws IOException, ServletException {
ServletInputStream in = Mockito.mock(ServletInputStream.class);
Mockito.when(request.getMethod()).thenReturn("PUT");
Mockito.when(request.getHeader("Transfer-Encoding")).thenReturn("chunked");
Mockito.when(request.getHeader("X-Expected-Entity-Length")).thenReturn("5");
Mockito.when(request.getInputStream()).thenReturn(in);
filter.doFilter(request, response, chain);
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
ServletInputStream wrappedIn = wrappedReq.getValue().getInputStream();
Mockito.when(in.isFinished()).thenReturn(false);
Assert.assertFalse(wrappedIn.isFinished());
Mockito.when(in.isReady()).thenReturn(true);
Assert.assertTrue(wrappedIn.isReady());
Mockito.when(in.read()).thenReturn(0xFF);
Assert.assertEquals(0xFF, wrappedIn.read());
Mockito.when(in.available()).thenReturn(100);
Assert.assertEquals(100, wrappedIn.available());
Mockito.when(in.skip(2)).thenReturn(2l);
Assert.assertEquals(2, wrappedIn.skip(2));
Mockito.when(in.read(Mockito.any(), Mockito.eq(0), Mockito.eq(100))).thenReturn(100);
Mockito.when(in.read(Mockito.any(), Mockito.eq(0), Mockito.eq(2))).thenReturn(2);
Assert.assertEquals(2, wrappedIn.read(new byte[100], 0, 100));
Mockito.when(in.read()).thenReturn(0xFF);
Assert.assertEquals(-1, wrappedIn.read());
Mockito.when(in.isFinished()).thenReturn(false);
Assert.assertTrue(wrappedIn.isFinished());
Mockito.when(in.isReady()).thenReturn(true);
Assert.assertFalse(wrappedIn.isReady());
}
}

View File

@@ -0,0 +1,162 @@
package org.cryptomator.webdav.filters;
import java.io.IOException;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.cryptomator.webdav.filters.UriNormalizationFilter.ResourceTypeChecker;
import org.cryptomator.webdav.filters.UriNormalizationFilter.ResourceTypeChecker.ResourceType;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
public class UriNormalizationFilterTest {
private ResourceTypeChecker resourceTypeChecker;
private UriNormalizationFilter filter;
private FilterChain chain;
private HttpServletRequest request;
private HttpServletResponse response;
@Before
public void setup() {
resourceTypeChecker = Mockito.mock(ResourceTypeChecker.class);
filter = new UriNormalizationFilter(resourceTypeChecker);
chain = Mockito.mock(FilterChain.class);
request = Mockito.mock(HttpServletRequest.class);
response = Mockito.mock(HttpServletResponse.class);
Mockito.when(resourceTypeChecker.typeOfResource("/file")).thenReturn(ResourceType.FILE);
Mockito.when(resourceTypeChecker.typeOfResource("/file/")).thenReturn(ResourceType.FILE);
Mockito.when(resourceTypeChecker.typeOfResource("/folder")).thenReturn(ResourceType.FOLDER);
Mockito.when(resourceTypeChecker.typeOfResource("/folder/")).thenReturn(ResourceType.FOLDER);
Mockito.when(resourceTypeChecker.typeOfResource("/404")).thenReturn(ResourceType.UNKNOWN);
Mockito.when(resourceTypeChecker.typeOfResource("/404/")).thenReturn(ResourceType.UNKNOWN);
}
/* FILE */
@Test
public void testFileRequest1() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/file");
Mockito.when(request.getRequestURI()).thenReturn("/file");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/file");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/file", wrappedReq.getValue().getRequestURI());
}
@Test
public void testFileRequest2() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/file/");
Mockito.when(request.getRequestURI()).thenReturn("/file/");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/file/");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/file", wrappedReq.getValue().getRequestURI());
}
@Test
public void testCopyFileRequest() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/file/");
Mockito.when(request.getRequestURI()).thenReturn("/file/");
Mockito.when(request.getMethod()).thenReturn("COPY");
Mockito.when(request.getHeader("Destination")).thenReturn("/404/");
filter.doFilter(request, response, chain);
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/404", wrappedReq.getValue().getHeader("Destination"));
}
/* FOLDER */
@Test
public void testFolderRequest1() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/folder");
Mockito.when(request.getRequestURI()).thenReturn("/folder");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/folder");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/folder/", wrappedReq.getValue().getRequestURI());
}
@Test
public void testFolderRequest2() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/folder/");
Mockito.when(request.getRequestURI()).thenReturn("/folder/");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/folder/");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/folder/", wrappedReq.getValue().getRequestURI());
}
@Test
public void testMoveFolderRequest() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/folder");
Mockito.when(request.getRequestURI()).thenReturn("/folder");
Mockito.when(request.getMethod()).thenReturn("MOVE");
Mockito.when(request.getHeader("Destination")).thenReturn("/404");
filter.doFilter(request, response, chain);
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/404/", wrappedReq.getValue().getHeader("Destination"));
}
/* UNKNOWN */
@Test
public void testUnknownPutRequest() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/404/");
Mockito.when(request.getRequestURI()).thenReturn("/404/");
Mockito.when(request.getMethod()).thenReturn("PUT");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/404/");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/404", wrappedReq.getValue().getRequestURI());
}
@Test
public void testUnknownMkcolRequest() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/404");
Mockito.when(request.getRequestURI()).thenReturn("/404");
Mockito.when(request.getMethod()).thenReturn("MKCOL");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/404");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertEquals("/404/", wrappedReq.getValue().getRequestURI());
}
@Test
public void testUnknownPropfindRequest() throws IOException, ServletException {
Mockito.when(request.getPathInfo()).thenReturn("/404");
Mockito.when(request.getRequestURI()).thenReturn("/404");
Mockito.when(request.getMethod()).thenReturn("PROPFIND");
filter.doFilter(request, response, chain);
Mockito.verify(resourceTypeChecker).typeOfResource("/404");
ArgumentCaptor<HttpServletRequest> wrappedReq = ArgumentCaptor.forClass(HttpServletRequest.class);
Mockito.verify(chain).doFilter(wrappedReq.capture(), Mockito.any(ServletResponse.class));
Assert.assertSame(request, wrappedReq.getValue());
}
}

View File

@@ -19,7 +19,7 @@ import javax.servlet.DispatcherType;
import org.cryptomator.filesystem.FileSystem;
import org.cryptomator.webdav.filters.AcceptRangeFilter;
import org.cryptomator.webdav.filters.LoggingHttpFilter;
import org.cryptomator.webdav.filters.PutIdleTimeoutFilter;
import org.cryptomator.webdav.filters.MacChunkedPutCompatibilityFilter;
import org.cryptomator.webdav.filters.UriNormalizationFilter;
import org.cryptomator.webdav.filters.UriNormalizationFilter.ResourceTypeChecker;
import org.cryptomator.webdav.filters.UriNormalizationFilter.ResourceTypeChecker.ResourceType;
@@ -69,7 +69,7 @@ class FileSystemBasedWebDavServer {
servletContext.addServlet(servletHolder, "/*");
servletContext.addFilter(AcceptRangeFilter.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(MacChunkedPutCompatibilityFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
servletContext.addFilter(LoggingHttpFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
servletCollection.mapContexts();