434810 better handling of bad messages

5 files changed, 137 insertions, 12 deletions

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
index 79f1c28d90..6f164ba226 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
@@ -443,13 +443,13 @@ public class HttpParser
             else if (ch==0)
                 break;
             else if (ch<0)
-                throw new BadMessage();
+                throw new BadMessage(-1);
             
             // count this white space as a header byte to avoid DOS
             if (_maxHeaderBytes>0 && ++_headerBytes>_maxHeaderBytes)
             {
                 LOG.warn("padding is too large >"+_maxHeaderBytes);
-                throw new BadMessage(HttpStatus.BAD_REQUEST_400);
+                throw new BadMessage(-1);
             }
         }
         return false;
@@ -1283,7 +1283,7 @@ public class HttpParser
                     if (_headerBytes>_maxHeaderBytes)
                     {
                         // Don't want to waste time reading data of a closed request
-                        throw new IllegalStateException("too much data after closed");
+                        throw new BadMessage(-1,"too much data after closed");
                     }
                 }
             }
@@ -1333,12 +1333,16 @@ public class HttpParser
         {
             BufferUtil.clear(buffer);
 
-            LOG.warn("badMessage: "+e._code+(e._message!=null?" "+e._message:"")+" for "+_handler);
+            if (e._code>0)
+                LOG.warn("badMessage: "+e._code+(e._message!=null?" "+e._message:"")+" for "+_handler);
+            else
+                LOG.warn("badMessage: "+(e._message!=null?e._message+" ":"")+"for "+_handler);
+                
             if (DEBUG)
                 LOG.debug(e);
             setState(State.CLOSED);
             _handler.badMessage(e._code, e._message);
-            return false;
+            return true;
         }
         catch(Exception e)
         {
@@ -1359,7 +1363,7 @@ public class HttpParser
                 setState(State.CLOSED);
             }
 
-            return false;
+            return true;
         }
     }
 
@@ -1621,7 +1625,9 @@ public class HttpParser
 
         /* ------------------------------------------------------------ */
         /** Called to signal that a bad HTTP message has been received.
-         * @param status The bad status to send
+         * @param status The bad status to send. If the status is <0, this indicates
+         * that the message was so bad that a response should not be sent and the 
+         * connection should be immediately closed.
          * @param reason The textual reason for badness
          */
         public void badMessage(int status, String reason);
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
index 7ff4d5a2a4..60353efd43 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -66,6 +66,7 @@ public class HttpChannelState
         WRITE_CALLBACK,   // handle an IO write callback
         READ_CALLBACK,    // handle an IO read callback
         WAIT,             // Wait for further events 
+        IO_WAIT,          // Wait for further IO 
         COMPLETE          // Complete the channel
     }
     
@@ -182,7 +183,7 @@ public class HttpChannelState
                     return Action.COMPLETE;
 
                 case COMPLETED:
-                    return Action.WAIT;
+                    return Action.IO_WAIT;
 
                 case ASYNC_WOKEN:
                     if (_asyncRead)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
index 16cbbf93d6..7f00e37ce6 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
@@ -474,8 +474,18 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
         @Override
         public void badMessage(int status, String reason)
         {
-            _generator.setPersistent(false);
-            super.badMessage(status,reason);
+            if (status<0)
+            {
+                getEndPoint().close();
+                _parser.atEOF();
+                LOG.info("Bad message close of "+getEndPoint());
+                completed();
+            }
+            else
+            {
+                _generator.setPersistent(false);
+                super.badMessage(status,reason);
+            }
         }
 
         @Override
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java
index c6c3eac0ec..7c997f8051 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java
@@ -40,6 +40,7 @@ import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.concurrent.Exchanger;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.servlet.ServletException;
@@ -137,6 +138,90 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
     * Feed a full header method
     */
     @Test
+    public void testFullWhite() throws Exception
+    {
+        configureServer(new HelloWorldHandler());
+
+        try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort()))
+        {
+            client.setSoTimeout(10000);
+            ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
+            ((StdErrLog) Log.getLogger(HttpConnection.class)).info("expect request is too large...");
+            OutputStream os = client.getOutputStream();
+
+            byte[] buffer = new byte[64 * 1024];
+            Arrays.fill(buffer, (byte)' ');
+
+            os.write(buffer);
+            os.flush();
+
+            // Read the close.
+            readClose(client);
+        }
+        finally
+        {
+            ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
+        }
+    }
+    
+
+    /*
+     * Feed a full header method
+     */
+    @Test
+    public void testFullWhiteAfter() throws Exception
+    {
+
+        configureServer(new HelloWorldHandler());
+
+        try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort()))
+        {
+            ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
+            ((StdErrLog)Log.getLogger(HttpConnection.class)).info("expect Bad Message close ...");
+            OutputStream os = client.getOutputStream();
+
+            byte[] buffer = new byte[64 * 1024];
+            buffer[0]='G';
+            buffer[1]='E';
+            buffer[2]='T';
+            buffer[3]=' ';
+            buffer[4]='/';
+            buffer[5]=' ';
+            buffer[6]='H';
+            buffer[7]='T';
+            buffer[8]='T';
+            buffer[9]='P';
+            buffer[10]='/';
+            buffer[11]='1';
+            buffer[12]='.';
+            buffer[13]='0';
+            buffer[14]='\n';
+            buffer[15]='\n';
+            Arrays.fill(buffer,16,buffer.length-1,(byte)' ');
+
+            os.write(buffer);
+            os.flush();
+
+            // Read the response.
+            long start = System.nanoTime();
+            String response = readResponse(client);
+            long end = System.nanoTime();
+
+            Assert.assertThat(response, Matchers.containsString("HTTP/1.1 200 OK"));
+            
+            Assert.assertThat(TimeUnit.NANOSECONDS.toSeconds(end-start),Matchers.lessThan(1L));
+            
+        }
+        finally
+        {
+            ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
+        }
+    }
+    
+    /*
+    * Feed a full header method
+    */
+    @Test
     public void testFullMethod() throws Exception
     {
         configureServer(new HelloWorldHandler());
@@ -1374,6 +1459,29 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
             throw e;
         }
     }
+    
+    /**
+     * Read Close.
+     *
+     * @param client Open client socket.
+     * @throws IOException in case of I/O problems
+     */
+    protected static void readClose(Socket client) throws IOException
+    {
+        try (BufferedReader br = new BufferedReader(new InputStreamReader(client.getInputStream())))
+        {
+            String line;
+
+            if ((line = br.readLine()) != null)
+                throw new IllegalStateException("unexpected data: "+line);
+
+            return;
+        }
+        catch (IOException e)
+        {
+            // expected
+        }
+    }
 
     protected void writeFragments(byte[] bytes, int[] points, StringBuilder message, OutputStream os) throws IOException, InterruptedException
     {
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java
index 6cc241a9b0..9a8ae62b3c 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SelectChannelServerSslTest.java
@@ -64,7 +64,7 @@ public class SelectChannelServerSslTest extends HttpServerTestBase
     public void testFullMethod() throws Exception
     {
     	// Don't run on Windows (buggy JVM)
-    	Assume.assumeTrue(!OS.IS_WINDOWS);
+    	// Assume.assumeTrue(!OS.IS_WINDOWS);
     	
         try
         {
@@ -80,7 +80,7 @@ public class SelectChannelServerSslTest extends HttpServerTestBase
     public void testFullURI() throws Exception
     {
     	// Don't run on Windows (buggy JVM)
-    	Assume.assumeTrue(!OS.IS_WINDOWS);
+    	// Assume.assumeTrue(!OS.IS_WINDOWS);
         try
         {
             super.testFullURI();