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

jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java

@@ -26,7 +26,7 @@ import org.eclipse.jetty.client.HttpExchange;
 import org.eclipse.jetty.client.HttpReceiver;
 import org.eclipse.jetty.client.HttpResponse;
 import org.eclipse.jetty.client.HttpResponseException;
-import org.eclipse.jetty.http.HttpCompliance;
+import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpParser;
@@ -325,13 +325,13 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
     }
 
     @Override
-    public void badMessage(int status, String reason)
+    public void badMessage(BadMessageException failure)
     {
         HttpExchange exchange = getHttpExchange();
         if (exchange != null)
         {
             HttpResponse response = exchange.getResponse();
-            response.status(status).reason(reason);
+            response.status(failure.getCode()).reason(failure.getReason());
             failAndClose(new HttpResponseException("HTTP protocol violation: bad response on " + getHttpConnection(), response));
         }
     }

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java

@@ -18,8 +18,6 @@
 
 package org.eclipse.jetty.client;
 
-import static org.junit.Assert.assertThat;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -1297,8 +1295,8 @@ public class HttpClientTest extends AbstractHttpClientServerTest
         }
         catch(ExecutionException e)
         {
-            assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
+            Assert.assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
-            assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
+            Assert.assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
         }
     }
 
@@ -1311,8 +1309,8 @@ public class HttpClientTest extends AbstractHttpClientServerTest
         }
         catch(ExecutionException e)
         {
-            assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
+            Assert.assertThat(e.getCause(), Matchers.instanceOf(BadMessageException.class));
-            assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
+            Assert.assertThat(e.getCause().getMessage(), Matchers.containsString("Unknown content"));
         }
     }
 

jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ResponseContentParser.java

@@ -305,9 +305,9 @@ public class ResponseContentParser extends StreamContentParser
         }
 
         @Override
-        public void badMessage(int status, String reason)
+        public void badMessage(BadMessageException failure)
         {
-            fail(new BadMessageException(status, reason));
+            fail(failure);
         }
 
         protected void fail(Throwable failure)

jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java

@@ -25,7 +25,9 @@ import java.util.concurrent.ConcurrentMap;
 import org.eclipse.jetty.fcgi.FCGI;
 import org.eclipse.jetty.fcgi.generator.Flusher;
 import org.eclipse.jetty.fcgi.parser.ServerParser;
+import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpField;
+import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.io.AbstractConnection;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.EndPoint;
@@ -187,9 +189,7 @@ public class ServerFCGIConnection extends AbstractConnection
             if (LOG.isDebugEnabled())
                 LOG.debug("Request {} failure on {}: {}", request, channel, failure);
             if (channel != null)
-            {
+                channel.onBadMessage(new BadMessageException(HttpStatus.BAD_REQUEST_400, null, failure));
-                channel.onBadMessage(400, failure.toString());
-            }
         }
     }
 }

jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java

@@ -1499,7 +1499,7 @@ public class HttpParser
                         if (DEBUG)
                             LOG.debug("{} EOF in {}",this,_state);
                         setState(State.CLOSED);
-                        _handler.badMessage(HttpStatus.BAD_REQUEST_400,null);
+                        _handler.badMessage(new BadMessageException(HttpStatus.BAD_REQUEST_400));
                         break;
                 }
             }
@@ -1525,7 +1525,7 @@ public class HttpParser
         if (_headerComplete)
             _handler.earlyEOF();
         else
-            _handler.badMessage(x._code, x._reason);
+            _handler.badMessage(x);
     }
 
     protected boolean parseContent(ByteBuffer buffer)
@@ -1804,10 +1804,20 @@ public class HttpParser
 
         /* ------------------------------------------------------------ */
         /** Called to signal that a bad HTTP message has been received.
-         * @param status The bad status to send
+         * @param failure the failure with the bad message information
-         * @param reason The textual reason for badness
          */
-        public void badMessage(int status, String reason);
+        public default void badMessage(BadMessageException failure)
+        {
+            badMessage(failure.getCode(), failure.getReason());
+        }
+
+        /**
+         * @deprecated use {@link #badMessage(BadMessageException)} instead
+         */
+        @Deprecated
+        public default void badMessage(int status, String reason)
+        {
+        }
 
         /* ------------------------------------------------------------ */
         /** @return the size in bytes of the per parser header cache

jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java

@@ -18,13 +18,6 @@
 
 package org.eclipse.jetty.http;
 
-import static org.hamcrest.Matchers.either;
-import static org.hamcrest.Matchers.equalTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -38,6 +31,12 @@ import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
+import static org.hamcrest.Matchers.either;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
 @RunWith(Parameterized.class)
 public class HttpGeneratorServerHTTPTest
 {
@@ -261,9 +260,9 @@ public class HttpGeneratorServerHTTPTest
         }
 
         @Override
-        public void badMessage(int status, String reason)
+        public void badMessage(BadMessageException failure)
         {
-            throw new IllegalStateException(reason);
+            throw failure;
         }
 
         @Override

jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java

@@ -18,8 +18,6 @@
 
 package org.eclipse.jetty.http;
 
-import static org.hamcrest.Matchers.contains;
-
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -34,6 +32,8 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hamcrest.Matchers.contains;
+
 public class HttpParserTest
 {
     static
@@ -2201,9 +2201,10 @@ public class HttpParserTest
         }
 
         @Override
-        public void badMessage(int status, String reason)
+        public void badMessage(BadMessageException failure)
         {
-            _bad = reason == null ? ("" + status) : reason;
+            String reason = failure.getReason();
+            _bad = reason == null ? String.valueOf(failure.getCode()) : reason;
         }
 
         @Override

jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java

@@ -408,9 +408,9 @@ public class HttpTester
         }
 
         @Override
-        public void badMessage(int status, String reason)
+        public void badMessage(BadMessageException failure)
         {
-            throw new RuntimeException(reason);
+            throw failure;
         }
 
         public ByteBuffer generate()

jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpChannelOverHTTP2.java

@@ -145,12 +145,12 @@ public class HttpChannelOverHTTP2 extends HttpChannel implements Closeable, Writ
         }
         catch (BadMessageException x)
         {
-            onBadMessage(x.getCode(), x.getReason());
+            onBadMessage(x);
             return null;
         }
         catch (Throwable x)
         {
-            onBadMessage(HttpStatus.INTERNAL_SERVER_ERROR_500, null);
+            onBadMessage(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, null, x));
             return null;
         }
     }
@@ -177,12 +177,12 @@ public class HttpChannelOverHTTP2 extends HttpChannel implements Closeable, Writ
         }
         catch (BadMessageException x)
         {
-            onBadMessage(x.getCode(), x.getReason());
+            onBadMessage(x);
             return null;
         }
         catch (Throwable x)
         {
-            onBadMessage(HttpStatus.INTERNAL_SERVER_ERROR_500, null);
+            onBadMessage(new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, null, x));
             return null;
         }
     }

jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java

@@ -527,12 +527,10 @@ public class SslConnection extends AbstractConnection
                     // don't bother writing, just notify of close
                     getWriteFlusher().onClose();
                 }
-                // Else,
                 else
                 {
-                    // try to flush what is pending
+                    // Try again
-                    // execute to avoid recursion
+                    _runCompleteWrite.run();
-                    getExecutor().execute(_runCompleteWrite);
                 }
             }
         }

jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java

@@ -514,40 +514,28 @@ public class IOTest
         assertThat(key,notNullValue());
         assertThat(selector.selectNow(), is(0));
         
-        client.write(BufferUtil.toBuffer("X"));
+        // Test wakeup before select
-        assertThat(selector.select(), is(1));
-        assertThat(key.readyOps(), is(SelectionKey.OP_READ));
-        assertThat(selector.selectedKeys(), Matchers.contains(key));
-
-        assertThat(selector.select(), is(0));
-        assertThat(key.readyOps(), is(SelectionKey.OP_READ));
-        assertThat(selector.selectedKeys(), Matchers.contains(key));
-
-        client.write(BufferUtil.toBuffer("X"));
-        selector.selectedKeys().clear();
-        assertThat(selector.select(), is(1));
-        assertThat(key.readyOps(), is(SelectionKey.OP_READ));
-        assertThat(selector.selectedKeys(), Matchers.contains(key));
-        
-        ByteBuffer buf = BufferUtil.allocate(1024);
-        int p = BufferUtil.flipToFill(buf);
-        assertThat(server.read(buf),is(2));
-        BufferUtil.flipToFlush(buf,p);
-        
         selector.wakeup();
-        selector.selectedKeys().clear();
         assertThat(selector.select(), is(0));
-        assertThat(selector.selectedKeys().size(),is(0));
         
-        client.write(BufferUtil.toBuffer("X"));
+        // Test wakeup after select
+        new Thread()
+        {
+            @Override
+            public void run()
+            {
+                try
+                {
+                    Thread.sleep(100);
                     selector.wakeup();
-        selector.selectedKeys().clear();
+                }
-        assertThat(selector.select(), is(1));
+                catch(Exception e)
-        assertThat(selector.selectedKeys().size(),is(1));
+                {
-        
+                    e.printStackTrace();
-        p = BufferUtil.flipToFill(buf);
+                }
-        assertThat(server.read(buf),is(1));
+            }
-        BufferUtil.flipToFlush(buf,p);
+            
-
+        }.start();
+        assertThat(selector.select(), is(0));
     }
 }

jetty-io/src/test/java/org/eclipse/jetty/io/IdleTimeoutTest.java

@@ -121,16 +121,18 @@ public class IdleTimeoutTest
     @Test
     public void testShorten() throws Exception
     {        
-        for (int i=0;i<20;i++)
+        _timeout.setIdleTimeout(2000);
+
+        for (int i=0;i<30;i++)
         {
-            Thread.sleep(200);
+            Thread.sleep(100);
             _timeout.notIdle();
         }
         Assert.assertNull(_expired);
         _timeout.setIdleTimeout(100);
         
         long start = System.nanoTime();
-        while (_expired==null && TimeUnit.NANOSECONDS.toSeconds(System.nanoTime()-start)<4)
+        while (_expired==null && TimeUnit.NANOSECONDS.toSeconds(System.nanoTime()-start)<5)
             Thread.sleep(200);
         
         Assert.assertNotNull(_expired);

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java

@@ -707,12 +707,14 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
         return _request.getHttpInput().earlyEOF();
     }
 
-    public void onBadMessage(int status, String reason)
+    public void onBadMessage(BadMessageException failure)
     {
+        int status = failure.getCode();
+        String reason = failure.getReason();
         if (status < 400 || status > 599)
-            status = HttpStatus.BAD_REQUEST_400;
+            failure = new BadMessageException(HttpStatus.BAD_REQUEST_400, reason, failure);
 
-        notifyRequestFailure(_request, new BadMessageException(status, reason));
+        notifyRequestFailure(_request, failure);
 
         Action action;
         try
@@ -721,10 +723,10 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
         }
         catch(IllegalStateException e)
         {
-            // The bad message cannot be handled in the current state, so throw
+            // The bad message cannot be handled in the current state,
-            // to hopefull somebody that can handle
+            // so rethrow, hopefully somebody will be able to handle.
             abort(e);
-            throw new BadMessageException(status,reason);
+            throw failure;
         }
 
         try

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java

@@ -269,7 +269,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
     }
 
     @Override
-    public void badMessage(int status, String reason)
+    public void badMessage(BadMessageException failure)
     {
         _httpConnection.getGenerator().setPersistent(false);
         try
@@ -283,7 +283,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
             LOG.ignore(e);
         }
 
-        onBadMessage(status, reason);
+        onBadMessage(failure);
     }
 
     @Override
@@ -333,7 +333,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
             {
                 if (_unknownExpectation)
                 {
-                    badMessage(HttpStatus.EXPECTATION_FAILED_417, null);
+                    badMessage(new BadMessageException(HttpStatus.EXPECTATION_FAILED_417));
                     return false;
                 }
 
@@ -374,7 +374,7 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
                         upgrade())
                     return true;
 
-                badMessage(HttpStatus.UPGRADE_REQUIRED_426, null);
+                badMessage(new BadMessageException(HttpStatus.UPGRADE_REQUIRED_426));
                 _httpConnection.getParser().close();
                 return false;
             }

jetty-server/src/main/java/org/eclipse/jetty/server/LocalConnector.java

@@ -471,11 +471,6 @@ public class LocalConnector extends AbstractConnector
                     return false;
                 }
                 
-                @Override
-                public void badMessage(int status, String reason)
-                {
-                }
-                
                 @Override
                 public boolean startResponse(HttpVersion version, int status, String reason)
                 {

jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java

@@ -448,6 +448,28 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
             interceptor=interceptor.getNextInterceptor();
         }
         
+        // Special handling for etags
+        for (ListIterator<HttpField> fields = baseRequest.getHttpFields().listIterator(); fields.hasNext();)
+        {
+            HttpField field = fields.next();
+            if (field.getHeader()==HttpHeader.IF_NONE_MATCH || field.getHeader()==HttpHeader.IF_MATCH)
+            {
+                String etag = field.getValue();
+                int i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote);
+                if (i>0)
+                {
+                    baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag",etag);
+                    while (i>=0)
+                    {
+                        etag=etag.substring(0,i)+etag.substring(i+CompressedContentFormat.GZIP._etag.length());
+                        i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote,i);
+                    }
+                    
+                    fields.set(new HttpField(field.getHeader(),etag));
+                }   
+            }
+        }
+        
         // If not a supported method - no Vary because no matter what client, this URI is always excluded
         if (!_methods.test(baseRequest.getMethod()))
         {
@@ -495,28 +517,6 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
             }
         }
       
-        // Special handling for etags
-        for (ListIterator<HttpField> fields = baseRequest.getHttpFields().listIterator(); fields.hasNext();)
-        {
-            HttpField field = fields.next();
-            if (field.getHeader()==HttpHeader.IF_NONE_MATCH || field.getHeader()==HttpHeader.IF_MATCH)
-            {
-                String etag = field.getValue();
-                int i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote);
-                if (i>0)
-                {
-                    baseRequest.setAttribute("o.e.j.s.h.gzip.GzipHandler.etag",etag);
-                    while (i>=0)
-                    {
-                        etag=etag.substring(0,i)+etag.substring(i+CompressedContentFormat.GZIP._etag.length());
-                        i=etag.indexOf(CompressedContentFormat.GZIP._etagQuote,i);
-                    }
-                    
-                    fields.set(new HttpField(field.getHeader(),etag));
-                }   
-            }
-        }
-      
         HttpOutput.Interceptor orig_interceptor = out.getInterceptor();
         try
         {

jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java

@@ -1,42 +0,0 @@
-//
-//  ========================================================================
-//  Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd.
-//  ------------------------------------------------------------------------
-//  All rights reserved. This program and the accompanying materials
-//  are made available under the terms of the Eclipse Public License v1.0
-//  and Apache License v2.0 which accompanies this distribution.
-//
-//      The Eclipse Public License is available at
-//      http://www.eclipse.org/legal/epl-v10.html
-//
-//      The Apache License v2.0 is available at
-//      http://www.opensource.org/licenses/apache2.0.php
-//
-//  You may elect to redistribute this code under either of these licenses.
-//  ========================================================================
-//
-
-package org.eclipse.jetty.server.handler.gzip;
-
-import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
-
-import java.util.Arrays;
-
-import org.junit.Test;
-
-public class GzipHandlerTest
-{
-    @Test
-    public void testAddGetPaths()
-    {
-        GzipHandler gzip = new GzipHandler();
-        gzip.addIncludedPaths("/foo");
-        gzip.addIncludedPaths("^/bar.*$");
-        
-        String[] includedPaths = gzip.getIncludedPaths();
-        assertThat("Included Paths.size", includedPaths.length, is(2));
-        assertThat("Included Paths", Arrays.asList(includedPaths), contains("/foo","^/bar.*$"));
-    }
-}

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java

@@ -47,13 +47,13 @@ import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.gzip.GzipHandler;
-import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.IO;
 import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+@SuppressWarnings("serial")
 public class GzipHandlerTest
 {
     private static final String __content =
@@ -151,6 +151,17 @@ public class GzipHandlerTest
                 writer.write(__content);
             }
         }
+
+        @Override
+        protected void doDelete(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
+        {        
+            String ifm = req.getHeader("If-Match"); 
+            if (ifm!=null && ifm.equals(__contentETag))
+                response.sendError(HttpServletResponse.SC_NO_CONTENT);
+            else
+                response.sendError(HttpServletResponse.SC_NOT_MODIFIED);
+        }
+        
     }
     
     public static class EchoServlet extends HttpServlet
@@ -347,6 +358,43 @@ public class GzipHandlerTest
         assertThat(response.get("ETag"),is(__contentETagGzip));
     }
     
+
+    @Test
+    public void testDeleteETagGzipHandler() throws Exception
+    {
+        HttpTester.Request request = HttpTester.newRequest();
+        HttpTester.Response response;
+
+        request.setMethod("DELETE");
+        request.setURI("/ctx/content");
+        request.setVersion("HTTP/1.0");
+        request.setHeader("Host","tester");
+        request.setHeader("If-Match","WrongEtag--gzip");
+        request.setHeader("accept-encoding","gzip");
+
+        response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
+        
+        assertThat(response.getStatus(),is(HttpServletResponse.SC_NOT_MODIFIED));
+        assertThat(response.get("Content-Encoding"),not(Matchers.equalToIgnoringCase("gzip")));
+        
+        
+        request = HttpTester.newRequest();
+        request.setMethod("DELETE");
+        request.setURI("/ctx/content");
+        request.setVersion("HTTP/1.0");
+        request.setHeader("Host","tester");
+        request.setHeader("If-Match",__contentETagGzip);
+        request.setHeader("accept-encoding","gzip");
+
+        response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
+        
+        assertThat(response.getStatus(),is(HttpServletResponse.SC_NO_CONTENT));
+        assertThat(response.get("Content-Encoding"),not(Matchers.equalToIgnoringCase("gzip")));
+    }
+    
+    
+    
+    
     @Test
     public void testForwardGzipHandler() throws Exception
     {