Bug 48839 - Header folding fails in NIO connector
Header folding fails in NIO connector
Status: RESOLVED FIXED
Product: Tomcat 6
Classification: Unclassified
Component: Connectors
6.0.20
PC Windows XP
: P2 normal (vote)
: default
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-03-02 08:05 UTC by Richa Baronia
Modified: 2010-05-14 15:24 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richa Baronia 2010-03-02 08:05:16 UTC
Overview : HTTP request having one of its header folded send to NIO connctor fails with 400 bad request.

Steps to Reproduce:
1) Send an HTTP request having a header whoes value is folded (example :
Add an HTTP header TestHeader  with value = "abcd" +"\r\n\t"+"efgh"
to a server which is using Java Nio Blocking Connector  Http11NioProtocol 

Expected Result : Request should be handled successfully

Actual Result   : 400 Bad Request response is send back

Additional Information: 
The issue is present in the  parseHeader() method of InternalNioInputBuffer class.
Here the variable headerParsePos is set according to the enum
HeaderParsePosition {HEADER_START, HEADER_NAME, HEADER_VALUE, HEADER_MULTI_LINE}
Inorder to verify if the header is multiline first the value of  headerParsePos is set to HEADER_MULTI_LINE
Then following code is executed
if ( headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE ) {
                if ( (chr != Constants.SP) && (chr != Constants.HT)) {
                    headerParsePos = HeaderParsePosition.HEADER_START;
                } 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++;
                }
}

Here if the first character of the next line is not equal to space or tab then the value of   variable   headerParsePos is set to HEADER_START
But if the value is equal to space or tab then value of   variable   headerParsePos remains HEADER_MULTI_LINE.
The futher parsing of the header value is done only in case headerParsePos is set to HEADER_VALUE
while (headerParsePos == HeaderParsePosition.HEADER_VALUE ||
              headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE) {
          if ( headerParsePos == HeaderParsePosition.HEADER_VALUE ) {
//code to parse header value 
         }
       if ( headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE ) {
//code to handle multiline header
        }
}
Since the value of headerParsePos is not being set back to HEADER_VALUE therefore code to handle multiline header is being executed .
And the while loop goes in an endless loop.
If the else part of code to handle multiline header if                     headerParsePos = HeaderParsePosition. HEADER_VALUE; line is added as below
if ( headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE ) {
                if ( (chr != Constants.SP) && (chr != Constants.HT)) {
                    headerParsePos = HeaderParsePosition.HEADER_START;
                } 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;
                }
}
Then the header folding is handled successfully
Comment 1 Mark Thomas 2010-03-29 10:40:19 UTC
Many thanks for the analysis and the suggested fix. Your fix has been applied to trunk (for 7.0.x) and has been proposed for 6.0.x.

I also created a unit test for this.
Comment 2 Mark Thomas 2010-05-14 15:24:35 UTC
This has been fixed in 6.0.x and will be included in 6.0.27 onwards.