Improved handling of return values from parser callback, by returning

 as early as possible to avoid race conditions with application code
 that may have returned true and reentered the parsing code.

1 files changed, 28 insertions, 20 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 8443f67de4..f16c4555e8 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
@@ -58,7 +58,7 @@ import org.eclipse.jetty.util.log.Logger;
  * For performance, the parse is heavily dependent on the 
  * {@link Trie#getBest(ByteBuffer, int, int)} method to look ahead in a
  * single pass for both the structure ( : and CRLF ) and semantic (which
- * header and value) of a header.  Specifically the static {@link HttpField#CACHE}
+ * header and value) of a header.  Specifically the static {@link HttpHeader#CACHE}
  * is used to lookup common combinations of headers and values 
  * (eg. "Connection: close"), or just header names (eg. "Connection:" ).
  * For headers who's value is not known statically (eg. Host, COOKIE) then a
@@ -185,7 +185,7 @@ public class HttpParser
             
             for (String charset : new String[]{"UTF-8","ISO-8859-1"})
             {
-                CACHE.put(field=new HttpGenerator.CachedHttpField(HttpHeader.CONTENT_TYPE,type+";charset="+charset));
+                CACHE.put(new HttpGenerator.CachedHttpField(HttpHeader.CONTENT_TYPE,type+";charset="+charset));
                 CACHE.put(new HttpGenerator.CachedHttpField(HttpHeader.CONTENT_TYPE,type+"; charset="+charset));
             }
         }
@@ -790,7 +790,7 @@ public class HttpParser
                 {
                     if (_valueString.endsWith(HttpHeaderValue.CHUNKED.toString()))
                         _endOfContent=EndOfContent.CHUNKED_CONTENT;
-                    else if (_valueString.indexOf(HttpHeaderValue.CHUNKED.toString()) >= 0)
+                    else if (_valueString.contains(HttpHeaderValue.CHUNKED.toString()))
                     {
                         throw new BadMessage(HttpStatus.BAD_REQUEST_400,"Bad chunking");
                     }
@@ -849,7 +849,7 @@ public class HttpParser
               
             case CONNECTION:
                 // Don't cache if not persistent
-                if (_valueString!=null && _valueString.indexOf("close")>=0)
+                if (_valueString!=null && _valueString.contains("close"))
                 {
                     _closed=true;
                     _connectionFields=null;
@@ -1200,8 +1200,6 @@ public class HttpParser
             LOG.debug("parseNext s={} {}",_state,BufferUtil.toDetailString(buffer));
         try
         {
-            boolean handle=false;
-            
             // Start a request/response
             if (_state==State.START)
             {
@@ -1210,28 +1208,39 @@ public class HttpParser
                 _methodString=null;
                 _endOfContent=EndOfContent.UNKNOWN_CONTENT;
                 _header=null;
-                handle=quickStart(buffer);
+                if (quickStart(buffer))
+                    return true;
             }
             
             // Request/response line
-            if (!handle && _state.ordinal()>= State.START.ordinal() && _state.ordinal()<State.HEADER.ordinal())
-                handle=parseLine(buffer);
+            if (_state.ordinal()>= State.START.ordinal() && _state.ordinal()<State.HEADER.ordinal())
+            {
+                if (parseLine(buffer))
+                    return true;
+            }
 
             // parse headers
-            if (!handle && _state.ordinal()>= State.HEADER.ordinal() && _state.ordinal()<State.CONTENT.ordinal())
-                handle=parseHeaders(buffer);
+            if (_state.ordinal()>= State.HEADER.ordinal() && _state.ordinal()<State.CONTENT.ordinal())
+            {
+                if (parseHeaders(buffer))
+                    return true;
+            }
             
             // parse content
-            if (!handle && _state.ordinal()>= State.CONTENT.ordinal() && _state.ordinal()<State.END.ordinal())
+            if (_state.ordinal()>= State.CONTENT.ordinal() && _state.ordinal()<State.END.ordinal())
             {
                 // Handle HEAD response
                 if (_responseStatus>0 && _headResponse)
                 {
                     setState(State.END);
-                    handle=_handler.messageComplete();
+                    if (_handler.messageComplete())
+                        return true;
                 }
                 else
-                    handle=parseContent(buffer);
+                {
+                    if (parseContent(buffer))
+                        return true;
+                }
             }
             
             // handle end states
@@ -1265,8 +1274,8 @@ public class HttpParser
                         break;
                         
                     case START:
-                        _handler.earlyEOF();
                         setState(State.CLOSED);
+                        _handler.earlyEOF();
                         break;
                         
                     case END:
@@ -1274,29 +1283,28 @@ public class HttpParser
                         break;
                         
                     case EOF_CONTENT:
-                        handle=_handler.messageComplete()||handle;
                         setState(State.CLOSED);
-                        break;
+                        return _handler.messageComplete();
 
                     case  CONTENT:
                     case  CHUNKED_CONTENT:
                     case  CHUNK_SIZE:
                     case  CHUNK_PARAMS:
                     case  CHUNK:
-                        _handler.earlyEOF();
                         setState(State.CLOSED);
+                        _handler.earlyEOF();
                         break;
 
                     default:
                         if (DEBUG)
                             LOG.debug("{} EOF in {}",this,_state);
-                        _handler.badMessage(400,null);
                         setState(State.CLOSED);
+                        _handler.badMessage(400,null);
                         break;
                 }
             }
             
-            return handle;
+            return false;
         }
         catch(BadMessage e)
         {