Author: kkolinko
Date: Thu Sep 29 10:32:40 2011
New Revision: 1177256

URL: http://svn.apache.org/viewvc?rev=1177256&view=rev
Log:
Merged revisions r1177152, r1177160 from tomcat/trunk:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51912
Fix several small issues in Header processing in Nio HTTP connector:
- Exception in skipLine() if debug logging is enabled.
- Several corner cases, identified through code review.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java
    
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Sep 29 10:32:40 2011
@@ -1 +1 @@
-/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096,1173241,1173256
 
,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177245
+/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096,1173241,1173256
 
,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java?rev=1177256&r1=1177255&r2=1177256&view=diff
==============================================================================
--- 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java 
(original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java 
Thu Sep 29 10:32:40 2011
@@ -340,11 +340,11 @@ public class InternalAprInputBuffer exte
 
             chr = buf[pos];
 
-            if ((chr == Constants.CR) || (chr == Constants.LF)) {
-                if (chr == Constants.LF) {
-                    pos++;
-                    return false;
-                }
+            if (chr == Constants.CR) {
+                // Skip
+            } else if (chr == Constants.LF) {
+                pos++;
+                return false;
             } else {
                 break;
             }

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java?rev=1177256&r1=1177255&r2=1177256&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java 
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java 
Thu Sep 29 10:32:40 2011
@@ -296,11 +296,11 @@ public class InternalInputBuffer extends
 
             chr = buf[pos];
 
-            if ((chr == Constants.CR) || (chr == Constants.LF)) {
-                if (chr == Constants.LF) {
-                    pos++;
-                    return false;
-                }
+            if (chr == Constants.CR) {
+                // Skip
+            } else if (chr == Constants.LF) {
+                pos++;
+                return false;
             } else {
                 break;
             }

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java?rev=1177256&r1=1177255&r2=1177256&view=diff
==============================================================================
--- 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java 
(original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java 
Thu Sep 29 10:32:40 2011
@@ -48,9 +48,49 @@ public class InternalNioInputBuffer exte
 
     // -------------------------------------------------------------- Constants
 
-    enum HeaderParseStatus {DONE, HAVE_MORE_HEADERS, NEED_MORE_DATA}
-    enum HeaderParsePosition {HEADER_START, HEADER_NAME, HEADER_VALUE,
-        HEADER_MULTI_LINE, HEADER_SKIPLINE}
+    enum HeaderParseStatus {
+        DONE, HAVE_MORE_HEADERS, NEED_MORE_DATA
+    }
+
+    enum HeaderParsePosition {
+        /**
+         * Start of a new header. A CRLF here means that there are no more
+         * headers. Any other character starts a header name.
+         */
+        HEADER_START,
+        /**
+         * Reading a header name. All characters of header are HTTP_TOKEN_CHAR.
+         * Header name is followed by ':'. No whitespace is allowed.<br />
+         * Any non-HTTP_TOKEN_CHAR (this includes any whitespace) encountered
+         * before ':' will result in the whole line being ignored.
+         */
+        HEADER_NAME,
+        /**
+         * Skipping whitespace before text of header value starts, either on 
the
+         * first line of header value (just after ':') or on subsequent lines
+         * when it is known that subsequent line starts with SP or HT.
+         */
+        HEADER_VALUE_START,
+        /**
+         * Reading the header value. We are inside the value. Either on the
+         * first line or on any subsequent line. We come into this state from
+         * HEADER_VALUE_START after the first non-SP/non-HT byte is encountered
+         * on the line.
+         */
+        HEADER_VALUE,
+        /**
+         * Before reading a new line of a header. Once the next byte is peeked,
+         * the state changes without advancing our position. The state becomes
+         * either HEADER_VALUE_START (if that first byte is SP or HT), or
+         * HEADER_START (otherwise).
+         */
+        HEADER_MULTI_LINE,
+        /**
+         * Reading all bytes until the next CRLF. The line is being ignored.
+         */
+        HEADER_SKIPLINE
+    }
+
     // ----------------------------------------------------------- Constructors
     
 
@@ -483,11 +523,11 @@ public class InternalNioInputBuffer exte
 
             chr = buf[pos];
 
-            if ((chr == Constants.CR) || (chr == Constants.LF)) {
-                if (chr == Constants.LF) {
-                    pos++;
-                    return HeaderParseStatus.DONE;
-                }
+            if (chr == Constants.CR) {
+                // Skip
+            } else if (chr == Constants.LF) {
+                pos++;
+                return HeaderParseStatus.DONE;
             } else {
                 break;
             }
@@ -500,14 +540,13 @@ public class InternalNioInputBuffer exte
             // Mark the current buffer position
             headerData.start = pos;
             headerParsePos = HeaderParsePosition.HEADER_NAME;
-        }    
+        }
 
         //
         // Reading the header name
         // Header name is always US-ASCII
         //
 
-
         while (headerParsePos == HeaderParsePosition.HEADER_NAME) {
 
             // Read new bytes if needed
@@ -517,29 +556,32 @@ public class InternalNioInputBuffer exte
                 }
             }
 
-            if (buf[pos] == Constants.COLON) {
-                headerParsePos = HeaderParsePosition.HEADER_VALUE;
+            chr = buf[pos];
+            if (chr == Constants.COLON) {
+                headerParsePos = HeaderParsePosition.HEADER_VALUE_START;
                 headerData.headerValue = headers.addValue(buf, 
headerData.start, pos - headerData.start);
-            } else if (!HTTP_TOKEN_CHAR[buf[pos]]) {
+                pos++;
+                // Mark the current buffer position
+                headerData.start = pos;
+                headerData.realPos = pos;
+                headerData.lastSignificantChar = pos;
+                break;
+            } else if (!HTTP_TOKEN_CHAR[chr]) {
                 // If a non-token header is detected, skip the line and
                 // ignore the header
+                headerData.lastSignificantChar = pos;
                 return skipLine();
             }
-            chr = buf[pos];
+
+            // chr is next byte of header name. Convert to lowercase.
             if ((chr >= Constants.A) && (chr <= Constants.Z)) {
                 buf[pos] = (byte) (chr - Constants.LC_OFFSET);
             }
-
             pos++;
-            if ( headerParsePos == HeaderParsePosition.HEADER_VALUE ) { 
-                // Mark the current buffer position
-                headerData.start = pos;
-                headerData.realPos = pos;
-            }
         }
 
-        
-        while (headerParsePos == HeaderParsePosition.HEADER_SKIPLINE) {
+        // Skip the line and ignore the header
+        if (headerParsePos == HeaderParsePosition.HEADER_SKIPLINE) {
             return skipLine();
         }
 
@@ -547,36 +589,34 @@ public class InternalNioInputBuffer exte
         // Reading the header value (which can be spanned over multiple lines)
         //
 
-        boolean eol = false;
-
-        while (headerParsePos == HeaderParsePosition.HEADER_VALUE ||
+        while (headerParsePos == HeaderParsePosition.HEADER_VALUE_START ||
+               headerParsePos == HeaderParsePosition.HEADER_VALUE ||
                headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE) {
-            if ( headerParsePos == HeaderParsePosition.HEADER_VALUE ) {
-            
-                boolean space = true;
 
+            if ( headerParsePos == HeaderParsePosition.HEADER_VALUE_START ) {
                 // Skipping spaces
-                while (space) {
-
+                while (true) {
                     // Read new bytes if needed
                     if (pos >= lastValid) {
                         if (!fill(true,false)) {//parse header 
-                            //HEADER_VALUE, should already be set
+                            //HEADER_VALUE_START
                             return HeaderParseStatus.NEED_MORE_DATA;
                         }
                     }
 
-                    if ((buf[pos] == Constants.SP) || (buf[pos] == 
Constants.HT)) {
+                    chr = buf[pos];
+                    if (chr == Constants.SP || chr == Constants.HT) {
                         pos++;
                     } else {
-                        space = false;
+                        headerParsePos = HeaderParsePosition.HEADER_VALUE;
+                        break;
                     }
-
                 }
-
-                headerData.lastSignificantChar = headerData.realPos;
+            }
+            if ( headerParsePos == HeaderParsePosition.HEADER_VALUE ) {
 
                 // Reading bytes until the end of the line
+                boolean eol = false;
                 while (!eol) {
 
                     // Read new bytes if needed
@@ -585,26 +625,26 @@ public class InternalNioInputBuffer exte
                             //HEADER_VALUE
                             return HeaderParseStatus.NEED_MORE_DATA;
                         }
-
                     }
 
-                    if (buf[pos] == Constants.CR) {
+                    chr = buf[pos];
+                    if (chr == Constants.CR) {
                         // Skip
-                    } else if (buf[pos] == Constants.LF) {
+                    } else if (chr == Constants.LF) {
                         eol = true;
-                    } else if (buf[pos] == Constants.SP) {
-                        buf[headerData.realPos] = buf[pos];
+                    } else if (chr == Constants.SP || chr == Constants.HT) {
+                        buf[headerData.realPos] = chr;
                         headerData.realPos++;
                     } else {
-                        buf[headerData.realPos] = buf[pos];
+                        buf[headerData.realPos] = chr;
                         headerData.realPos++;
                         headerData.lastSignificantChar = headerData.realPos;
                     }
 
                     pos++;
-
                 }
 
+                // Ignore whitespaces at the end of the line
                 headerData.realPos = headerData.lastSignificantChar;
 
                 // Checking the first character of the new line. If the 
character
@@ -624,18 +664,19 @@ public class InternalNioInputBuffer exte
             if ( headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE ) {
                 if ( (chr != Constants.SP) && (chr != Constants.HT)) {
                     headerParsePos = HeaderParsePosition.HEADER_START;
+                    break;
                 } else {
-                    eol = false;
                     // Copying one extra space in the buffer (since there must
                     // be at least one space inserted between the lines)
                     buf[headerData.realPos] = chr;
                     headerData.realPos++;
-                    headerParsePos = HeaderParsePosition.HEADER_VALUE;
+                    headerParsePos = HeaderParsePosition.HEADER_VALUE_START;
                 }
             }
         }
         // Set the header value
-        headerData.headerValue.setBytes(buf, headerData.start, 
headerData.realPos - headerData.start);
+        headerData.headerValue.setBytes(buf, headerData.start,
+                headerData.lastSignificantChar - headerData.start);
         headerData.recycle();
         return HeaderParseStatus.HAVE_MORE_HEADERS;
     }
@@ -681,9 +722,32 @@ public class InternalNioInputBuffer exte
 
     private HeaderParseData headerData = new HeaderParseData();
     public static class HeaderParseData {
+        /**
+         * When parsing header name: first character of the header.<br />
+         * When skipping broken header line: first character of the header.<br 
/>
+         * When parsing header value: first character after ':'.
+         */
         int start = 0;
+        /**
+         * When parsing header name: not used (stays as 0).<br />
+         * When skipping broken header line: not used (stays as 0).<br />
+         * When parsing header value: starts as the first character after ':'.
+         * Then is increased as far as more bytes of the header are harvested.
+         * Bytes from buf[pos] are copied to buf[realPos]. Thus the string from
+         * [start] to [realPos-1] is the prepared value of the header, with
+         * whitespaces removed as needed.<br />
+         */
         int realPos = 0;
+        /**
+         * When parsing header name: not used (stays as 0).<br />
+         * When skipping broken header line: last non-CR/non-LF character.<br 
/>
+         * When parsing header value: position after the last not-LWS 
character.<br />
+         */
         int lastSignificantChar = 0;
+        /**
+         * MB that will store the value of the header. It is null while parsing
+         * header name and is created after the name has been parsed.
+         */
         MessageBytes headerValue = null;
         public void recycle() {
             start = 0;

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1177256&r1=1177255&r2=1177256&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Sep 29 10:32:40 2011
@@ -62,6 +62,10 @@
         the socket from 30s to 10s. (kkolinko)
       </fix>
       <fix>
+        <bug>51912</bug>: Fix HTTP header processing in NIO HTTP connector.
+        (kkolinko)
+      </fix>
+      <fix>
         Improve MimeHeaders.toString(). (kkolinko)
       </fix>
     </changelog>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to