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
|