tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remy Maucherat <r...@apache.org>
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 11:44:17 GMT
On Wed, 2008-02-27 at 18:25 -0700, Filip Hanik - Dev Lists wrote:
> 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.

I don't see anything wrong with fixing the thing.

> 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.

I agree, but I don't see this is being proposed. It could be too risky
obviously.

Rémy



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


Mime
View raw message