Remy Maucherat wrote:
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.
not really, the patch was fairly large, and I was against for the reason
of regression when the patch becomes a redo/refactor of how the logic
works. especially when there is a much simpler fix that does the exact
thing.
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/B2CConverter.java?r1=572497&r2=572496&pathrev=572497
and the comment that went along with the checkin
"The patch I just committed is still a bit fragile (since it depends on
InputBuffer knowing what CharChunk does internally)"
at this point I don't see how this has to be argued again. the series of
events speak for themselves.
And Remy, I appreciate how you've handled this, you've kept your cool, I
had a rough day yesterday, and I apologize if I snapped in my email. Not
my intention.
Filip
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
