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 Mon, 03 Mar 2008 22:56:52 GMT
Mark Thomas wrote:
> 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/InputBuffer.java
>>>>> ===================================================================
>>>>> ---java/org/apache/catalina/connector/InputBuffer.java    
>>>>> (revision 630535)
>>>>> +++java/org/apache/catalina/connector/InputBuffer.java    (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 
>>> http://issues.apache.org/bugzilla/show_bug.cgi?id=44494  ?
>> 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).
the original patch didn't do that

-            while( true ) { // conv.ready() ) {
+            while( iis.available()>0 ) { // conv.ready() ) {
                 int cnt=conv.read( result, 0, BUFFER_SIZE );
                 if( cnt <= 0 ) {
                     // End of stream ! - we may be in a bad state
@@ -251,7 +251,12 @@
     public  final int read() throws IOException {
         return (pos < end ) ? (buf[pos++] & 0xff) : -1;
     }
+    
+    public int available() throws IOException {
+        return end-pos;

the API change is because a new API was introduced in the refactoring

Filip
+    }


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


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


Mime
View raw message