443049 Improved HttpParser illegal character messages

2 files changed, 113 insertions, 57 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 f54d02b973..f1932eb972 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
@@ -18,6 +18,11 @@
 
 package org.eclipse.jetty.http;
 
+import static org.eclipse.jetty.http.HttpTokens.CARRIAGE_RETURN;
+import static org.eclipse.jetty.http.HttpTokens.LINE_FEED;
+import static org.eclipse.jetty.http.HttpTokens.SPACE;
+import static org.eclipse.jetty.http.HttpTokens.TAB;
+
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 
@@ -365,22 +370,22 @@ public class HttpParser
         
         if (_cr)
         {
-            if (ch!=HttpTokens.LINE_FEED)
+            if (ch!=LINE_FEED)
                 throw new BadMessage("Bad EOL");
             _cr=false;
             return ch;
         }
 
-        if (ch>=0 && ch<HttpTokens.SPACE)
+        if (ch>=0 && ch<SPACE)
         {
-            if (ch==HttpTokens.CARRIAGE_RETURN)
+            if (ch==CARRIAGE_RETURN)
             {
                 if (buffer.hasRemaining())
                 {
                     if(_maxHeaderBytes>0 && _state.ordinal()<State.END.ordinal())
                         _headerBytes++;
                     ch=buffer.get();
-                    if (ch!=HttpTokens.LINE_FEED)
+                    if (ch!=LINE_FEED)
                         throw new BadMessage("Bad EOL");
                 }
                 else
@@ -392,8 +397,8 @@ public class HttpParser
                 }
             }
             // Only LF or TAB acceptable special characters
-            else if (!(ch==HttpTokens.LINE_FEED || ch==HttpTokens.TAB))
-                throw new BadMessage("Illegal character");
+            else if (!(ch==LINE_FEED || ch==TAB))
+                throw new IllegalCharacter(ch,buffer);
         }
         
         return ch;
@@ -433,7 +438,7 @@ public class HttpParser
         {
             int ch=next(buffer);
 
-            if (ch > HttpTokens.SPACE)
+            if (ch > SPACE)
             {
                 _string.setLength(0);
                 _string.append((char)ch);
@@ -508,7 +513,7 @@ public class HttpParser
             switch (_state)
             {
                 case METHOD:
-                    if (ch == HttpTokens.SPACE)
+                    if (ch == SPACE)
                     {
                         _length=_string.length();
                         _methodString=takeString();
@@ -517,8 +522,13 @@ public class HttpParser
                             _methodString=method.asString();
                         setState(State.SPACE1);
                     }
-                    else if (ch < HttpTokens.SPACE)
-                        throw new BadMessage(ch<0?"Illegal character":"No URI");
+                    else if (ch < SPACE)
+                    {
+                        if (ch==LINE_FEED)
+                            throw new BadMessage("No URI");
+                        else
+                            throw new IllegalCharacter(ch,buffer);
+                    }
                     else
                         _string.append((char)ch);
                     break;
@@ -534,7 +544,7 @@ public class HttpParser
                         setState(State.SPACE1);
                     }
                     else if (ch < HttpTokens.SPACE)
-                        throw new BadMessage(ch<0?"Illegal character":"No Status");
+                        throw new IllegalCharacter(ch,buffer);
                     else
                         _string.append((char)ch);
                     break;
@@ -1147,8 +1157,8 @@ public class HttpParser
                             _length=_string.length();
                         break;
                     }
-                     
-                    throw new BadMessage("Illegal character");
+
+                    throw new IllegalCharacter(ch,buffer);
 
                 case HEADER_VALUE:
                     if (ch>HttpTokens.SPACE || ch<0)
@@ -1172,8 +1182,8 @@ public class HttpParser
                         setState(State.HEADER);
                         break; 
                     }
-
-                    throw new BadMessage("Illegal character");
+                    
+                    throw new IllegalCharacter(ch,buffer);
 
                 case HEADER_IN_VALUE:
                     if (ch>=HttpTokens.SPACE || ch<0 || ch==HttpTokens.TAB)
@@ -1201,7 +1211,8 @@ public class HttpParser
                         setState(State.HEADER);
                         break;
                     }
-                    throw new BadMessage("Illegal character");
+
+                    throw new IllegalCharacter(ch,buffer);
                     
                 default:
                     throw new IllegalStateException(_state.toString());
@@ -1578,6 +1589,29 @@ public class HttpParser
     }
 
     /* ------------------------------------------------------------------------------- */
+    public Trie<HttpField> getFieldCache()
+    {
+        return _connectionFields;
+    }
+
+    /* ------------------------------------------------------------------------------- */
+    private String getProxyField(ByteBuffer buffer)
+    {
+        _string.setLength(0);
+        _length=0;
+        
+        while (buffer.hasRemaining())
+        {
+            // process each character
+            byte ch=next(buffer);
+            if (ch<=' ')
+                return _string.toString();
+            _string.append((char)ch);    
+        }
+        throw new BadMessage();
+    }
+    
+    /* ------------------------------------------------------------------------------- */
     @Override
     public String toString()
     {
@@ -1632,11 +1666,17 @@ public class HttpParser
         public int getHeaderCacheSize();
     }
 
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
     public interface ProxyHandler 
     {
         void proxied(String protocol, String sAddr, String dAddr, int sPort, int dPort);
     }
-    
+
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
     public interface RequestHandler<T> extends HttpHandler<T>
     {
         /**
@@ -1657,6 +1697,9 @@ public class HttpParser
         public abstract boolean parsedHostHeader(String host,int port);
     }
 
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
     public interface ResponseHandler<T> extends HttpHandler<T>
     {
         /**
@@ -1665,24 +1708,14 @@ public class HttpParser
         public abstract boolean startResponse(HttpVersion version, int status, String reason);
     }
 
-    public Trie<HttpField> getFieldCache()
-    {
-        return _connectionFields;
-    }
-
-    private String getProxyField(ByteBuffer buffer)
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
+    /* ------------------------------------------------------------------------------- */
+    private class IllegalCharacter extends BadMessage
     {
-        _string.setLength(0);
-        _length=0;
-        
-        while (buffer.hasRemaining())
+        IllegalCharacter(byte ch,ByteBuffer buffer)
         {
-            // process each character
-            byte ch=next(buffer);
-            if (ch<=' ')
-                return _string.toString();
-            _string.append((char)ch);    
+            super(String.format("Illegal character 0x%x in state=%s in '%s'",ch,_state,BufferUtil.toDebugString(buffer)));
         }
-        throw new BadMessage();
     }
 }
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java
index 53d24db966..7f0b785a8e 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java
@@ -920,6 +920,11 @@ public class BufferUtil
         return builder.toString();
     }
 
+    /* ------------------------------------------------------------ */
+    /** Convert Buffer to a detail debug string of pointers and content
+     * @param buffer
+     * @return A string showing the pointers and content of the buffer
+     */
     public static String toDetailString(ByteBuffer buffer)
     {
         if (buffer == null)
@@ -942,15 +947,33 @@ public class BufferUtil
         buf.append(buffer.remaining());
         buf.append("]={");
 
+        appendDebugString(buf,buffer);
+
+        buf.append("}");
+
+        return buf.toString();
+    }
+
+    /* ------------------------------------------------------------ */
+    /** Convert buffer to a Debug String.
+     * @param buffer
+     * @return A string showing the escaped content of the buffer around the
+     * position and limit (marked with &lt;&lt;&lt; and &gt;&gt;&gt;)
+     */
+    public static String toDebugString(ByteBuffer buffer)
+    {
+        if (buffer == null)
+            return "null";
+        StringBuilder buf = new StringBuilder();
+        appendDebugString(buf,buffer);
+        return buf.toString();
+    }
+    
+    private static void appendDebugString(StringBuilder buf,ByteBuffer buffer)
+    {
         for (int i = 0; i < buffer.position(); i++)
         {
-            char c = (char)buffer.get(i);
-            if (c >= ' ' && c <= 127)
-                buf.append(c);
-            else if (c == '\r' || c == '\n')
-                buf.append('|');
-            else
-                buf.append('\ufffd');
+            appendContentChar(buf,buffer.get(i));
             if (i == 16 && buffer.position() > 32)
             {
                 buf.append("...");
@@ -960,13 +983,7 @@ public class BufferUtil
         buf.append("<<<");
         for (int i = buffer.position(); i < buffer.limit(); i++)
         {
-            char c = (char)buffer.get(i);
-            if (c >= ' ' && c <= 127)
-                buf.append(c);
-            else if (c == '\r' || c == '\n')
-                buf.append('|');
-            else
-                buf.append('\ufffd');
+            appendContentChar(buf,buffer.get(i));
             if (i == buffer.position() + 16 && buffer.limit() > buffer.position() + 32)
             {
                 buf.append("...");
@@ -978,13 +995,7 @@ public class BufferUtil
         buffer.limit(buffer.capacity());
         for (int i = limit; i < buffer.capacity(); i++)
         {
-            char c = (char)buffer.get(i);
-            if (c >= ' ' && c <= 127)
-                buf.append(c);
-            else if (c == '\r' || c == '\n')
-                buf.append('|');
-            else
-                buf.append('\ufffd');
+            appendContentChar(buf,buffer.get(i));
             if (i == limit + 16 && buffer.capacity() > limit + 32)
             {
                 buf.append("...");
@@ -992,11 +1003,23 @@ public class BufferUtil
             }
         }
         buffer.limit(limit);
-        buf.append("}");
-
-        return buf.toString();
     }
 
+    private static void appendContentChar(StringBuilder buf, byte b)
+    {
+        if (b == '\\')
+            buf.append("\\\\");   
+        else if (b >= ' ')
+            buf.append((char)b);
+        else if (b == '\r')
+            buf.append("\\r");
+        else if (b == '\n')
+            buf.append("\\n");
+        else if (b == '\t')
+            buf.append("\\t");
+        else
+            buf.append("\\x").append(TypeUtil.toHexString(b));
+    }
 
     private final static int[] decDivisors =
             {1000000000, 100000000, 10000000, 1000000, 100000, 10000, 1000, 100, 10, 1};