Merge branch 'jetty-9.3.x' of https://github.com/eclipse/jetty.project into jetty-9.3.x

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)

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<E> implements Iterable<MappedResource<E>>, Dumpable
         mappings.clear();
     }
     
+    public void removeIf(Predicate<MappedResource<E>> predicate)
+    {
+        mappings.removeIf(predicate);
+    }
+    
     /**
      * Return a list of MappedResource matches for the specified path.
      * 

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<SslHandshakeListener> 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
+                        // Extra check on unwrapResultStatus == OK with zero bytes consumed
-                        // to SSL client on android (see bug #454773)
+                        // 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;
+                        _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;

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"+
+                "Host: whatever\r\n"+
-        "Content-Type: text/plane\r\n"+
+                "Content-Type: text/plane\r\n"+
-        "Content-Length: "+10+"\r\n"+
+                "Content-Length: "+10+"\r\n"+
-        "\r\n"+
+                "\r\n"+
-        "0123456789\r\n"+
+                "0123456789\r\n"+
-        "GET / HTTP/1.1\r\n"+
+                "GET / HTTP/1.1\r\n"+
-        "Host: whatever\r\n"+
+                "Host: whatever\r\n"+
-        "Content-Type: text/plane\r\n"+
+                "Content-Type: text/plane\r\n"+
-        "Content-Length: "+10+"\r\n"+
+                "Content-Length: "+10+"\r\n"+
-        "Connection: close\r\n"+
+                "Connection: close\r\n"+
-        "\r\n"+
+                "\r\n"+
-        "ABCDEFGHIJ\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

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.
+     * <p>
+     *     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.
+     * </p>
      *
      * @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() + "]";
+        }
+    }
 }

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<WebSocketFrame> 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<WebSocketFrame> 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)));
+        }
+    }
 }

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