tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1344253 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/ajp/AjpAprProcessor.java webapps/docs/changelog.xml
Date Tue, 05 Jun 2012 12:34:02 GMT
2012/6/4 Mark Thomas <markt@apache.org>:
> On 04/06/2012 07:41, Konstantin Kolinko wrote:
>> 2012/5/30  <markt@apache.org>:
>>> Author: markt
>>> Date: Wed May 30 13:35:55 2012
>>> New Revision: 1344253
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1344253&view=rev
>>> Log:
>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53119
>>> Make sure the buffer is cleared on any error to prevent any possible overflow
if it is written to again before the connection is closed.
>>> I can't reproduce the error with the provided test case but based on code inspection
this should fix it.
>>>
>>> Modified:
>>>    tomcat/tc7.0.x/trunk/   (props changed)
>>>    tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
>>>    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>>>
>>> Propchange: tomcat/tc7.0.x/trunk/
>>> ------------------------------------------------------------------------------
>>>  Merged /tomcat/trunk:r1344250
>>>
>>> Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
>>> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1344253&r1=1344252&r2=1344253&view=diff
>>> ==============================================================================
>>> --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original)
>>> +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Wed
May 30 13:35:55 2012
>>> @@ -288,6 +288,9 @@ public class AjpAprProcessor extends Abs
>>>
>>>         if (outputBuffer.position() > 0) {
>>>             if ((socketRef != 0) && Socket.sendbb(socketRef, 0,
outputBuffer.position()) < 0) {
>>> +                // There are no re-tries so clear the buffer to prevent
a
>>> +                // possible overflow if the buffer is used again. BZ53119.
>>> +                outputBuffer.clear();
>>>                 throw new IOException(sm.getString("ajpprocessor.failedsend"));
>>>             }
>>>             outputBuffer.clear();
>>>
>>
>> Looks good. Backport to 6.0?
>
> I haven't looked at the 6.0.x code to see if the exact same code path is
> possible but a back port wouldn't do any harm in this case and is
> probably quicker than working out if the issue can occur.
>

Proposed for 6.0. I had to prepare a patch, because affected code is
located in different method.

>> AjpNioProcessor#output(byte[], int, int) seems to have the same issue.
>
> I'm not sure. The OP that saw the error with APR/native could not
> recreate it with NIO or BIO. That said, looking at the code there are
> certainly a few ways the write buffer can't be cleared. The worst case
> is an error message in the logs so I a not too concerned at this point.
>

I commented in bugzilla and fixed for NIO in trunk.
I proposed for 7.0 instead of applying immediately because the issue
is hard to observe and I think the change is worth reviewing.

Best regards,
Konstantin Kolinko

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


Mime
View raw message