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 Thu, 28 Feb 2008 01:25:34 GMT
Remy Maucherat wrote:
> On Wed, 2008-02-27 at 17:08 -0700, Filip Hanik - Dev Lists wrote:
>   
>> Remy Maucherat wrote:
>>     
>>> This makes me think there's a straight issue in CharChunk (I suppose the
>>> problem is that realReadChars gets called with len = 0; if it handled
>>> that scenario, things should be fine).
>>>   
>>>       
>> the old char chunk worked just fine, so why not stick with it. the error 
>> above is a result of B2CConverter and InputBuffer being modified
>>     
>
> Ok, but the new code looks a bit nicer, 
huh? here we go again, first it was "elegant", now "it's nice". how 
about "it works" as argument.
bottom line is, my patch was very simple, the B2C Converter is using an 
intermediary inputstream, but wasn't looking if there was data available 
to read, so that

take a look at the original patch proposed
http://people.apache.org/~fhanik/tomcat/b2c/patch.txt

obviously, my patch touches nothing of what you claim to be "my code", 
all I did today was, revert the old patch, and apply the simple patch.
and it works.

> so I would prefer keeping it if
> possible. From what I can see, your code replaces the limit value (the
> one which would cause the problem if it was 0) by the amount of bytes
> which have been read in the ByteChunk. This would translate to:
> 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();
>
> The problem is that I sort of like the idea of the limit (it's related
> to the space left in the char buffer for adding the parsed chars). So
> maybe trying to respect that with a decent fallback could work (didn't
> try it):
> 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, (len == 0) ? bb.getLength() : len);
>          bb.setOffset(bb.getEnd());
>  
>          return cb.getLength();
>   
so now we're trying to "fix the fix". why is that? I do NOT claim that 
"my shit don't stink" but lets face it, the broken patch came with such 
claim. and now were arguing that it's "nicer".
sorry for being blunt, but lets cut to the chase here, what is it that 
you are really trying to say and maybe we can focus the technical 
discussion on that, instead of running around in circles.

Obviously the whole byte/char-chunk code is very sensitive, since 
several components keep their own offset/end/limit flags to the same 
byte[], which makes it somewhat of a difficult thing to read into. and 
that's why I managed to come up with a patch that didn't upset that eco 
system of the old code.


Filip

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


Mime
View raw message