diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e139741721..3858e9bd5cb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,12 +43,12 @@ Contact the project developers via the project's "dev" list. Search for bugs ---------------- -This project uses Bugzilla to track ongoing development and issues. +This project uses GitHub Issues to track ongoing development and issues. -- [https://bugs.eclipse.org/bugs/buglist.cgi?product=Jetty](https://bugs.eclipse.org/bugs/buglist.cgi?product=Jetty) +- [https://github.com/eclipse/jetty.project/issues](https://github.com/eclipse/jetty.project/issues) Create a new bug ----------------- Be sure to search for existing bugs before you create another one. Remember that contributions are always welcome! -- [https://bugs.eclipse.org/bugs/enter_bug.cgi?product=Jetty](https://bugs.eclipse.org/bugs/enter_bug.cgi?product=Jetty) +- [https://github.com/eclipse/jetty.project/issues](https://github.com/eclipse/jetty.project/issues) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java index d1c36cfbd69..e618213e758 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.function.Predicate; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; @@ -69,6 +70,11 @@ public class PathMappings implements Iterable>, Dumpable mappings.clear(); } + public void removeIf(Predicate> predicate) + { + mappings.removeIf(predicate); + } + /** * Return a list of MappedResource matches for the specified path. * diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 5319ae5e96f..65813b610a5 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -79,8 +79,6 @@ import org.eclipse.jetty.util.log.Logger; public class SslConnection extends AbstractConnection { private static final Logger LOG = Log.getLogger(SslConnection.class); - private static final ByteBuffer __FILL_CALLED_FLUSH= BufferUtil.allocate(0); - private static final ByteBuffer __FLUSH_CALLED_FILL= BufferUtil.allocate(0); private final List handshakeListeners = new ArrayList<>(); private final ByteBufferPool _bufferPool; @@ -569,9 +567,10 @@ public class SslConnection extends AbstractConnection HandshakeStatus unwrapHandshakeStatus = unwrapResult.getHandshakeStatus(); Status unwrapResultStatus = unwrapResult.getStatus(); - // Extra check on unwrapResultStatus == OK with zero length buffer is due - // to SSL client on android (see bug #454773) - _underFlown = unwrapResultStatus == Status.BUFFER_UNDERFLOW || unwrapResultStatus == Status.OK && unwrapResult.bytesConsumed()==0 && unwrapResult.bytesProduced()==0; + // Extra check on unwrapResultStatus == OK with zero bytes consumed + // or produced is due to an SSL client on Android (see bug #454773). + _underFlown = unwrapResultStatus == Status.BUFFER_UNDERFLOW || + unwrapResultStatus == Status.OK && unwrapResult.bytesConsumed() == 0 && unwrapResult.bytesProduced() == 0; if (_underFlown) { @@ -665,15 +664,17 @@ public class SslConnection extends AbstractConnection { // If we are called from flush() // return to let it do the wrapping. - if (buffer == __FLUSH_CALLED_FILL) + if (_flushRequiresFillToProgress) return 0; _fillRequiresFlushToProgress = true; - flush(__FILL_CALLED_FLUSH); + flush(BufferUtil.EMPTY_BUFFER); if (BufferUtil.isEmpty(_encryptedOutput)) { - // The flush wrote all the encrypted bytes so continue to fill + // The flush wrote all the encrypted bytes so continue to fill. _fillRequiresFlushToProgress = false; + if (_underFlown) + break decryption; continue; } else @@ -896,11 +897,11 @@ public class SslConnection extends AbstractConnection case NEED_UNWRAP: // Ah we need to fill some data so we can write. // So if we were not called from fill and the app is not reading anyway - if (appOuts[0]!=__FILL_CALLED_FLUSH && !getFillInterest().isInterested()) + if (!_fillRequiresFlushToProgress && !getFillInterest().isInterested()) { // Tell the onFillable method that there might be a write to complete _flushRequiresFillToProgress = true; - fill(__FLUSH_CALLED_FILL); + fill(BufferUtil.EMPTY_BUFFER); // Check if after the fill() we need to wrap again if (_sslEngine.getHandshakeStatus() == HandshakeStatus.NEED_WRAP) continue; diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 25bee9611ef..392b8311ad3 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -31,6 +31,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.BufferedReader; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileReader; import java.io.IOException; @@ -46,6 +47,7 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import javax.servlet.DispatcherType; import javax.servlet.MultipartConfigElement; import javax.servlet.ServletException; import javax.servlet.ServletInputStream; @@ -55,9 +57,12 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.Part; +import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.toolchain.test.FS; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiPartInputStreamParser; import org.eclipse.jetty.util.Utf8Appendable; @@ -827,6 +832,78 @@ public class RequestTest } + @Test + @Ignore("See issue #1175") + public void testMultiPartFormDataReadInputThenParams() throws Exception + { + final File tmpdir = MavenTestingUtils.getTargetTestingDir("multipart"); + FS.ensureEmpty(tmpdir); + + Handler handler = new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, + ServletException + { + if (baseRequest.getDispatcherType() != DispatcherType.REQUEST) + return; + + // Fake a MultiPartConfig'd servlet endpoint + MultipartConfigElement multipartConfig = new MultipartConfigElement(tmpdir.getAbsolutePath()); + request.setAttribute(Request.__MULTIPART_CONFIG_ELEMENT, multipartConfig); + + // Normal processing + baseRequest.setHandled(true); + + // Fake the commons-fileupload behavior + int length = request.getContentLength(); + InputStream in = request.getInputStream(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + IO.copy(in, out, length); // commons-fileupload does not read to EOF + + LOG.info("input stream = " + in); + + // Record what happened as servlet response headers + response.setIntHeader("x-request-content-length", request.getContentLength()); + response.setIntHeader("x-request-content-read", out.size()); + String foo = request.getParameter("foo"); // uri query parameter + String bar = request.getParameter("bar"); // form-data content parameter + response.setHeader("x-foo", foo == null ? "null" : foo); + response.setHeader("x-bar", bar == null ? "null" : bar); + } + }; + _server.stop(); + _server.setHandler(handler); + _server.start(); + + String multipart = "--AaBbCc\r\n"+ + "content-disposition: form-data; name=\"bar\"\r\n"+ + "\r\n"+ + "BarContent\r\n"+ + "--AaBbCc\r\n"+ + "content-disposition: form-data; name=\"stuff\"\r\n"+ + "Content-Type: text/plain;charset=ISO-8859-1\r\n"+ + "\r\n"+ + "000000000000000000000000000000000000000000000000000\r\n"+ + "--AaBbCc--\r\n"; + + String request="POST /?foo=FooUri HTTP/1.1\r\n"+ + "Host: whatever\r\n"+ + "Content-Type: multipart/form-data; boundary=\"AaBbCc\"\r\n"+ + "Content-Length: "+multipart.getBytes().length+"\r\n"+ + "Connection: close\r\n"+ + "\r\n"+ + multipart; + + + HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request)); + + // It should always be possible to read query string + assertThat("response.x-foo", response.get("x-foo"), is("FooUri")); + // Not possible to read request content parameters? + assertThat("response.x-bar", response.get("x-bar"), is("null")); // TODO: should this work? + } + @Test public void testPartialRead() throws Exception { @@ -843,35 +920,35 @@ public class RequestTest response.getOutputStream().write(b); response.flushBuffer(); } - + }; _server.stop(); _server.setHandler(handler); _server.start(); - + String request="GET / HTTP/1.1\r\n"+ - "Host: whatever\r\n"+ - "Content-Type: text/plane\r\n"+ - "Content-Length: "+10+"\r\n"+ - "\r\n"+ - "0123456789\r\n"+ - "GET / HTTP/1.1\r\n"+ - "Host: whatever\r\n"+ - "Content-Type: text/plane\r\n"+ - "Content-Length: "+10+"\r\n"+ - "Connection: close\r\n"+ - "\r\n"+ - "ABCDEFGHIJ\r\n"; - + "Host: whatever\r\n"+ + "Content-Type: text/plane\r\n"+ + "Content-Length: "+10+"\r\n"+ + "\r\n"+ + "0123456789\r\n"+ + "GET / HTTP/1.1\r\n"+ + "Host: whatever\r\n"+ + "Content-Type: text/plane\r\n"+ + "Content-Length: "+10+"\r\n"+ + "Connection: close\r\n"+ + "\r\n"+ + "ABCDEFGHIJ\r\n"; + String responses = _connector.getResponses(request); - + int index=responses.indexOf("read="+(int)'0'); assertTrue(index>0); - + index=responses.indexOf("read="+(int)'A',index+7); assertTrue(index>0); } - + @Test public void testQueryAfterRead() throws Exception diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java index aac64ec26ba..beea529e118 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java @@ -60,7 +60,7 @@ public class NativeWebSocketConfiguration extends ContainerLifeCycle implements @Override public void doStop() throws Exception { - mappings.reset(); + mappings.removeIf((mapped) -> !(mapped.getResource() instanceof PersistedWebSocketCreator)); super.doStop(); } @@ -111,13 +111,23 @@ public class NativeWebSocketConfiguration extends ContainerLifeCycle implements /** * Manually add a WebSocket mapping. + *

+ * If mapping is added before this configuration is started, then it is persisted through + * stop/start of this configuration's lifecycle. Otherwise it will be removed when + * this configuration is stopped. + *

* * @param pathSpec the pathspec to respond on * @param creator the websocket creator to activate on the provided mapping. */ public void addMapping(PathSpec pathSpec, WebSocketCreator creator) { - mappings.put(pathSpec, creator); + WebSocketCreator wsCreator = creator; + if (!isRunning()) + { + wsCreator = new PersistedWebSocketCreator(creator); + } + mappings.put(pathSpec, wsCreator); } /** @@ -170,4 +180,26 @@ public class NativeWebSocketConfiguration extends ContainerLifeCycle implements } }); } + + private class PersistedWebSocketCreator implements WebSocketCreator + { + private final WebSocketCreator delegate; + + public PersistedWebSocketCreator(WebSocketCreator delegate) + { + this.delegate = delegate; + } + + @Override + public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse resp) + { + return delegate.createWebSocket(req, resp); + } + + @Override + public String toString() + { + return "Persisted[" + super.toString() + "]"; + } + } } diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java index fabffcc3405..131b289a9b6 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java @@ -34,6 +34,7 @@ import javax.servlet.DispatcherType; import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.toolchain.test.EventQueue; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; @@ -166,14 +167,44 @@ public class WebSocketUpgradeFilterTest NativeWebSocketConfiguration configuration = new NativeWebSocketConfiguration(context.getServletContext()); configuration.getFactory().getPolicy().setMaxTextMessageSize(10 * 1024 * 1024); configuration.addMapping(new ServletPathSpec("/info/*"), infoCreator); - context.getServletContext().setAttribute(NativeWebSocketConfiguration.class.getName(), configuration); + context.setAttribute(NativeWebSocketConfiguration.class.getName(), configuration); server.start(); return server; } }}); - + + // Embedded WSUF, added as filter, apply app-ws configuration via wsuf constructor + + cases.add(new Object[]{"wsuf/addFilter/WSUF Constructor configure", new ServerProvider() + { + @Override + public Server newServer() throws Exception + { + Server server = new Server(); + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + server.setHandler(context); + + NativeWebSocketConfiguration configuration = new NativeWebSocketConfiguration(context.getServletContext()); + configuration.getFactory().getPolicy().setMaxTextMessageSize(10 * 1024 * 1024); + configuration.addMapping(new ServletPathSpec("/info/*"), infoCreator); + context.addBean(configuration, true); + + FilterHolder wsufHolder = new FilterHolder(new WebSocketUpgradeFilter(configuration)); + context.addFilter(wsufHolder, "/*", EnumSet.of(DispatcherType.REQUEST)); + + server.start(); + + return server; + } + }}); + // Embedded WSUF, added as filter, apply app-ws configuration via ServletContextListener cases.add(new Object[]{"wsuf.configureContext/ServletContextListener configure", new ServerProvider() @@ -291,7 +322,7 @@ public class WebSocketUpgradeFilterTest } @Test - public void testConfiguration() throws Exception + public void testNormalConfiguration() throws Exception { URI destUri = serverUri.resolve("/info/"); @@ -311,4 +342,45 @@ public class WebSocketUpgradeFilterTest assertThat("payload", payload, containsString("session.maxTextMessageSize=" + (10 * 1024 * 1024))); } } + + @Test + public void testStopStartOfHandler() throws Exception + { + URI destUri = serverUri.resolve("/info/"); + + try (BlockheadClient client = new BlockheadClient(destUri)) + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + client.write(new TextFrame().setPayload("hello 1")); + + EventQueue frames = client.readFrames(1, 1000, TimeUnit.MILLISECONDS); + String payload = frames.poll().getPayloadAsUTF8(); + + // If we can connect and send a text message, we know that the endpoint was + // added properly, and the response will help us verify the policy configuration too + assertThat("payload", payload, containsString("session.maxTextMessageSize=" + (10 * 1024 * 1024))); + } + + server.getHandler().stop(); + server.getHandler().start(); + + try (BlockheadClient client = new BlockheadClient(destUri)) + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + client.write(new TextFrame().setPayload("hello 2")); + + EventQueue frames = client.readFrames(1, 1000, TimeUnit.MILLISECONDS); + String payload = frames.poll().getPayloadAsUTF8(); + + // If we can connect and send a text message, we know that the endpoint was + // added properly, and the response will help us verify the policy configuration too + assertThat("payload", payload, containsString("session.maxTextMessageSize=" + (10 * 1024 * 1024))); + } + } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java index a9b817663c0..49898a4d0c1 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java @@ -33,6 +33,7 @@ import java.util.stream.IntStream; import javax.servlet.ServletException; import javax.servlet.ServletInputStream; import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -505,6 +506,40 @@ public class HttpClientTest extends AbstractTest Assert.assertEquals(1, completes.get()); } + @Test + public void testHEADResponds200() throws Exception + { + testHEAD(servletPath, HttpStatus.OK_200); + } + + @Test + public void testHEADResponds404() throws Exception + { + testHEAD("/notMapped", HttpStatus.NOT_FOUND_404); + } + + private void testHEAD(String path, int status) throws Exception + { + byte[] data = new byte[1024]; + new Random().nextBytes(data); + start(new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + response.getOutputStream().write(data); + } + }); + + ContentResponse response = client.newRequest(newURI()) + .method(HttpMethod.HEAD) + .path(path) + .send(); + + Assert.assertEquals(status, response.getStatus()); + Assert.assertEquals(0, response.getContent().length); + } + private void sleep(long time) throws IOException { try