Re: seems dblib can get TDS protocol wrong in unusual case

Hayes, Ted (London) wrote:
> 
> The problem is that dbsqlok() processes the TDS_DONEPROC_RESULT and with
> no error flag set in done_flags, sets dbproc->dbresults_state to
> _DB_RES_SUCCEED and exits.  This just makes the subsequent dbresults()
> invocation exit immediately, and the results from the second half of the
> packet are lost.  From the TDS specification, it seems to me that maybe
> the TDS_DONE_MORE_RESULTS bit in done_flags should cause dbsqlok() to
> continue looping to make further tds_process_tokens() invocations.
> 
> I added this at line 4601 of dblib.c
> 
> if (done_flags & TDS_DONE_MORE_RESULTS)
> { break; }
> 
> /* else continue with existing code..*/
> tdsdump_log(...
> dbproc->dbresults_state = _DB_RES_SUCCEED;
> 
> With this change, dbsqlok() carries on and processes additional tokens
> including the 0x7c (PROCID) at offset 0x171, the 0xa0 (COLNAME) at 0x17a
> and 0xa1 (COLFMT) at 0x188.  dbsqlok() exits when the 0xae result token
> is encountered at 0x191 and dbresults() then successfully processes the
> rest of the packet.

My, what a can of worms you've opened.  

For the nonce I've improved/broken dbsqlok with this:

retcode = (done_flags & TDS_DONE_ERROR)? FAIL : SUCCEED;
dbproc->dbresults_state = (done_flags & TDS_DONE_MORE_RESULTS)?
	_DB_RES_NEXT_RESULT : _DB_RES_NO_MORE_RESULTS;
return retcode;

The code you found in 0.82 is decidedly odd.  The dbresults_state is set
to rows/norows in the fail branch but only to succeed in the succeessful
branch.  

But with the above, even if I include logic to continue gobbling packets
(along the lines you suggest), 5 unit tests fail.  You might try your
patch and see.  Specifically, t0018.c.  

Now, it might be that the test is bad.  It looks that way to me.  But I
think I've reached a state of _DB_RES_VERY_CONFUSED, so I'm checking in
the broken code and taking a break.  

Thanks for pointing this out.  Results processing is, er, important.  

Regards, 

--jkl