[seam-issues] [JBoss JIRA] Commented: (JBSEAM-3897) MultipartRequest Error: 100% CPU (for 2.0.2.GA) or Missing data (for trunk)

[Jindrich Vimr (JIRA)]

    [ https://jira.jboss.org/browse/JBSEAM-3897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12561092#action_12561092 ] 

Jindrich Vimr commented on JBSEAM-3897:
---------------------------------------

I've come to the same problems - depending on POST data, the parser either loops  or fails to recognize some parameters.
There are two different bugs in the implementation:

1) parameter header/value boundary being missed (and thus two parameters mixed together) while looking for CRLF+CRLF between parameter header (name) and it's value. If  end of  the 2048 byte internal buffer fits into this two CRLF's, all following lines (up to double CRLF in next parameter) are parsed as next parameter header lines, and next parameter's value is stored as as this parameter value.

2) endless loop when parameter header line is 2048 bytes long.  This is very rare, but can be triggered with problem 1), i.e. due to previous problem parameter value is parsed as parameter header. This problem is partially solved using loop count, but this unfortunately leads to all following parameters not being parsed. No data gets read to the buffer and the buffer is parsed and parsed in endless loop.

Solutions:
1) check if there is a room for next CRLF in the buffer, if not, let the parser read more data

2) if there is no space left in the buffer for more data, reallocate it.

Please find attached unittests + data for the tests. These tests triggers both problems.

There is also a patch for MultipartRequestImpl.java, and fixed version of if. 





> MultipartRequest Error: 100% CPU (for 2.0.2.GA) or Missing data (for trunk)
> ---------------------------------------------------------------------------
>
>                 Key: JBSEAM-3897
>                 URL: https://jira.jboss.org/browse/JBSEAM-3897
>             Project: Seam
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.0.2.GA, The future
>         Environment: Seam 2.0.2.GA
>            Reporter: Bryan Brouckaert
>            Priority: Critical
>         Attachments: 20101103-multipart-request-impl-test.zip, MultipartRequest.java, MultipartRequestImpl.java, MultipartRequestImpl.java.diff
>
>
> There is a fundamental error in MultipartRequest that causes 100% CPU usage in a very specific case.  I had it for version 2.0.2.GA, but I saw in the code that the issue is still in the trunk.  In the trunk version it will not caus 100% CPU, but the rest of the request will be ignored.
> The errors that cause it
> --------------------------------
> 1) If a multipart header is longer then 2K, the class will at a certain point start reading blocks of 0 bytes.  It does this because it only reads the amound of bytes that are still free in the buffer.  Because the buffer is only 2 K big and a header must be entirely in the buffer in order to be processed, this can be 0 bytes if the header is bigger then 2 K.  The trunk solves this by using a loop counter, while it should test if there is still place in the buffer before reading that amount of bytes.  Of course, 2 K headers should not occur, but it should be detected.
> 2) The code assumes that the CR-LF that devides the data and the headers are in the same block of the buffer as the header itself.  If this isn't the case (you must be unlucky for this to happen) the emtpy line isn't treated as the divider between headers and data but as a header itself.  Because the empty lines is treated as a header, the data below it is also treated as a header.  Because data can easly be more then 2 K, you get quickly into error 1.
> I had a case where the first 2K of the request is always the same for a specific user.  Unfortuantely, the only part that did not fit in the first 2K was the viewstate, which is easly bigger then 2K.  Because the boundary has a random number, the size varried slightly and for about 50% of the requests made by a certain user we got the above problem.  The only thing the "fix" of the trunk would have done, is throwing an error instaid of using 100% CPU but the user would have been very unhapy.
> My solution to the problem
> ------------------------------------
> I did not have time to wait for a fix, I do use seam for a very critical healthcare platform, I fixed it myself.
> 1) Dynamicly growning buffers in case the headers are bigger then 2K.  At 128 K I throw an error, no header should be longer then that (probably that is more then frendly enough)
> 2) Check if a header isn't an empty line, if so threat it as the devider between the header and data.
> Some closure notes
> ----------------------------
> I included the patched version (2.0.2.GA) that I'm using now.  My unlucky user confirmed that it solved the problem, all his files where uploaded without any problem.
> The loopcount is a good idea, it is always possible that a stream keeps returning 0 bytes even if you request more then that.  But I assure you, that was not the problem, the problem was that the code only requested 0 bytes.  I have a 1.8 G logfile to prove it, and I will send it to you if you don't beleave me ;-)
> I had to add some TRACE info in order find the solution.  Since nobody in his right mind has trace on by default I left it in.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira