Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51912

Fix several small issues in Header processing in Nio HTTP connector:
- Split HEADER_VALUE state into HEADER_VALUE_START and HEADER_VALUE.
The first one skips leading whitespace, the second one - does not.
- Trim trailing tabs as well as spaces. RFC-2616 Ch.4.2 says that both are LWS and both can be ignored.
- Always trim whitespace from last line.
- Fix initial value of lastSignificantChar when calling skipLine(). It fixes BZ 51912.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1177160 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
index a310dbb..79f99a8 100644 (file)
--- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
@@ -66,16 +66,23 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
          */
         HEADER_NAME,
         /**
-         * Reading the header value. We come into this state by two ways:<br />
-         * a) just after the ':' on the first line of the header, b) on the
-         * start of a new line when it is known that it starts with SP or HT.
+         * 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 (if that first byte is SP or HT), or HEADER_START
-         * (otherwise).
+         * either HEADER_VALUE_START (if that first byte is SP or HT), or
+         * HEADER_START (otherwise).
          */
         HEADER_MULTI_LINE,
         /**
@@ -549,25 +556,28 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
                 }
             }
 
-            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;
-            }
         }
 
         // Skip the line and ignore the header
@@ -579,36 +589,34 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
         // 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
@@ -617,26 +625,26 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
                             //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
@@ -656,18 +664,19 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
             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;
     }
@@ -715,21 +724,24 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
     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.
+         * whitespaces removed as needed.<br />
          */
         int realPos = 0;
         /**
          * When parsing header name: not used (stays as 0).<br />
-         * When parsing header value: position after the last not-LWS character.
+         * 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;
         /**