tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: Bug in B2C converter WAS: svn commit: r568307 - /tomcat/trunk/java/org/apache/tomcat/util/buf/
Date Mon, 03 Mar 2008 21:13:03 GMT
Filip Hanik - Dev Lists wrote:
> Mark Thomas wrote:
>> Remy Maucherat wrote:
>>> On Thu, 2008-02-28 at 01:36 +0100, Remy Maucherat wrote:
>>>> Index: java/org/apache/catalina/connector/
>>>> ===================================================================
>>>> ---java/org/apache/catalina/connector/    (revision 
>>>> 630535)
>>>> +++java/org/apache/catalina/connector/    (working 
>>>> copy)
>>>> @@ -355,7 +355,7 @@
>>>>          }
>>>>          state = CHAR_STATE;
>>>> -        conv.convert(bb, cb, len);
>>>> +        conv.convert(bb, cb, bb.getLength());
>>>>          bb.setOffset(bb.getEnd());
>>>>          return cb.getLength();
>>> This seems to work for me. It should be equivalent to the value returned
>>> by the available method, since the value passed is the same (it is the
>>> amount of bytes which have been read).
>> Just to be clear, do you mean that the above patch 
>>  ?
> that's correct
> Filip

First off - sorry it has taken me a few days to reply.

Secondly, I still haven't had enough time to look at this in detail so my 
reasoning may be way off base. It it is, I apologise - feel free to correct 
me with or without flames.

Looking through the InputBuffer code, realReadBytes(), realReadChars() and 
realWriteChars() all ignore the parameters passed in with the exception of 
len in realReadChars() which the patch above changes so it too is ignored.

It appears to me that all of these methods need to be modified not to take 
any parameters since they all operate on the internal buffers rather than 
the ones passed to them (a job for another day). On this basis, the patch 
above patch strikes me as the right way forward rather than the one 
currently applied to trunk.

On the basis of the above, I would vote -0 for the current patch since I 
don't think I have done enough research to vote -1 or +1.

However, since the current patch removes a method from a public API I will 
be voting -1 solely on that basis. Once that is fixed, I'll change my vote 
to -0 until I have a change to look at this some more (hopefully in the 
next week or so).


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message