tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: Bug in B2C converter WAS: svn commit: r568307 - /tomcat/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java
Date Wed, 27 Feb 2008 23:01:59 GMT
Remy Maucherat wrote:
> On Wed, 2008-02-27 at 12:51 -0700, Filip Hanik - Dev Lists wrote:
>   
>> Bill Barker wrote:
>>     
>>> "Remy Maucherat" <remm@apache.org> wrote in message 
>>> news:46CB7F51.1030201@apache.org...
>>>   
>>>       
>>>> Filip Hanik - Dev Lists wrote:
>>>>     
>>>>         
>>>>> Test Case and 5.5.x patch can be found here.
>>>>> http://people.apache.org/~fhanik/tomcat/b2c/
>>>>>
>>>>> This is what is happening
>>>>>
>>>>> int cnt=conv.read( result, 0, BUFFER_SIZE );
>>>>> is called with a "while (true)" statement,
>>>>>
>>>>> When the IntermediateInputStream.read returns -1, the above
>>>>>           
>> statement 
>>     
>>>>> returns cnt==1.
>>>>> So to avoid calling conv.read, we must check to see if we have
>>>>>           
>> more bytes 
>>     
>>>>> to read by implementing the available() method, to avoid the
>>>>>           
>> inputstream 
>>     
>>>>> ever returning -1.
>>>>>       
>>>>>           
>>>> It's possible, but I have a hard time understanding the issue.
>>>>
>>>>     
>>>>         
>>> The issue is that InputStreamReader reads 8192 bytes from 
>>> IntermediateInputStream on the first go.  It then translates them
>>>       
>> into 2734 
>>     
>>> chars, but thinks that the last few bytes represent an incomplete
>>>       
>> char, so 
>>     
>>> holds onto them.  On the next call, IntermediateInputStream returns
>>>       
>> -1, so 
>>     
>>> InputStreamReader outputs the last char as best it can (resulting
>>>       
>> in 
>>     
>>> returning 1).  Then the IntermediateInputStream buffer is reset, and
>>>       
>> it can 
>>     
>>> continue on reading (but from the wrong position, resulting in
>>>       
>> corruption).
>>     
>>> Filip's patch is inelegant (better would be to use the ByteChunk
>>>       
>> sink), but 
>>     
>>> other than my looking for a better way to do it, I can't come up
>>>       
>> with the 
>>     
>>> required technical reason to porting the base of it to 5.5 (of
>>>       
>> course, I 
>>     
>>> could care less what he does in his sandbox :).
>>>   
>>>       
>> unfortunately, the "elegant" solution caused a regression :(
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=44494
>> I tested this with the inelegant (original proposed) patch and it
>> worked 
>> fine, so I'm gonna fix this in trunk and propose the patch to 6.0
>>     
>
> Ok, and there's no time to think ? A patch must be applied within the
> next 5 seconds ? (probably because it was not your patch, I assume :) )
>   
the original patch was 4 lines of code change, that was considered 
inelegant (as a matter of fact, it was dissed without even being looked 
at), instead we made a more complex patch that caused regression. I 
simply suggest that the simpler patch is easier to work with, and that 
we make that available to the folks that want to patch their own version.
and available it is.

Filip

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


Mime
View raw message