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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]