db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Army <qoz...@sbcglobal.net>
Subject Re: [jira] Updated: (DERBY-125) Network Server can send DSS greater than 32K to client, which breaks DRDA protocol.
Date Tue, 10 Jan 2006 18:11:15 GMT
Bryan,

Thanks as usual for the incredibly detailed explanations--I don't know how long 
it took you to write that email, but it was extremely helpful!

Regarding the test cases for DERBY-125 and DERBY-170, I'll need to spend some 
time looking at those before I can come up with any suggestions.  In the 
meantime, here's some quick feedback based from my first time through the email...

> 1) Should this be one patch or several? 

As others on the list have also replied, if it makes sense to split the patch up 
into pieces, then that's definitely the preferred way to go.  It sounds doable 
w.r.t to DERBY-125 and DERBY-170, so I'd say go ahead and break those two parts 
out, as Knut suggested.

> 2) Can we make the tests better? Yes, and I'll be working on this,
> but I think I need more help from you, particularly regarding the
> off-by-one problem in finalizeDSSLength, which I discuss at length
> below. This is the problem child in the testing area, and I need
> some good ideas for how to make the tests better.

I'll have to reply to this one later; need to investigate more...

> 3) Is the DERBY-170 test specific to the JCC client? Yes, this test is
> specific to the JCC client, and, more specifically, it is specific to
> the "deferPrepares" behavior of the JCC client.

[ snip ]

> Do you know of a way to cause the skipRemainder() processing to be run
> on a arbitrarily large object, other than the case mentioned in the bug 
> script?

Thanks for explaining this one.  As for an alternative, I will again have to 
think about this some more before I can make any suggestions.

> 4) What's going on with this off-by-one bug, and how can we write a test
> which clearly demonstrates the bug? [ ... ] After you read this, hopefully 
> you will not only understand why the test that I added doesn't show the 
> bug, but also will be able to suggest how I could write a test that *does* 
> show the bug.

Yes, I can see now why the test that you added doesn't show the bug--this is a 
great explanation of the problem, and I appreciate you taking the time to write 
it up.  I'll try to go through this sometime in the next day or two and will let 
you know if anything comes to mind (I have a lot going on right now, so I can't 
get to this immediately--but I'll try to do it soon).

<snip: excellent description of the off-by-one problem>

> This makes me wonder whether it would be reasonable to modify 
> ensureLength() so that it pre-filled the buffer with non-zero data 
> (for example, 0xFC), so that the bug would be more likely to be 
> obvious, since it would end up placing 0xFC into the last byte of 
> the message, rather than 00 in most cases.

My gut reaction is that we probably don't want to do that--it seems like a bit 
too much for the sake of testing a single Jira issue.

> I don't think we'd always want ensureLength() to do this, for performance 
> reasons, but perhaps there is some way to conditionally compile such code 
> into the test version of DDMWriter?

There may be, but I think we should avoid doing different compiles for testing 
vs. development environments.  Any tests we run should occur against the 
compiled codeline as it will be released, or else we end up testing code that is 
not released, and not testing code that is released, which seems like a bad idea 
to me.

> However, I do believe that the bug is real, for two reasons:

I definitely agree that it's a bug--I didn't mean to call that into question.  I 
was just perplexed as to why it didn't manifest itself.  You've now explained 
that part to me, so the trick is to write an appropriate test case.

> I'm afraid that by now I've gone on much too long.

Details are great--especialy when they're as well thought-out and clearly 
written as you tend to give.

>  - after considering my explanation of the bug, do you have an idea of a
>    better way to write a test for it? If not, should I just remove the
>    DERBY-125 test from prepStmt.java?

Not yet; I'll have to think about this some more.  Note that if you decide to 
break your patch up into the separate issues, you could go ahead and submit 
patches for DERBY-491 and DERBY-492 right now, and then submit the other patches 
(DERBY-125 and DERBY-170) when the test cases have been finalized.  And as for 
DERBY-170, since there's at least one case where the test fails without the 
patch--namely, against JCC--I think it'd be okay to commit that change, as well; 
when a better test that shows the problem against the Derby client is available, 
you could submit that test case as a follow-up patch.

>  - Can you try the experiment of applying all of my patches *except* for
>    the off-by-one bug (just apply everything, then remove the "+1" from
>    finalizeDSSLength), and then run
> 
>     java -Dframework=DerbyNetClient
>        org.apache.derby.functionTests.harness.RunTest lang/procedure.java
> 
>    My belief is that you will get an error in the client from the
>    parseFastVCMorVCS routine in NetStatementReply.java saying that
> 
>    "only one of the VCM, VCS length can be greater than 0". That, if you
>    can get it, is proof of the off-by-one bug causing damage.

I will try this out and see what happens.  In the meantime, thanks again for 
being so thorough with all of this.  I'll post again when I have further 
feedback to give...

Army

Mime
View raw message