Re: Old dblib.c bug still there

2016-06-15 16:37 GMT+01:00 Nem W Schlecht <[email protected]>:
> Well, I use defncopy all the time. :)  Glad we worked through this.  Should
> I mail this to Freddy directly or is attaching it here (as I've done) good
> enough?
>

Merged with a comment for overflow.

Frediano

> On Wed, Jun 15, 2016 at 3:37 AM, Thompson, William <[email protected]
>> wrote:
>
>> Hi.
>>
>> Glad you found a solution!
>> From looking at the code, I can see why that would be just what was needed!
>> The STRINGBIND is basically saying "don't strip any trailing blanks off
>> the returned string"
>> Passing 0 as the size of the destination field is basically telling dbbind
>> "don't worry, I've got enough space to take whatever you're giving me,
>> just give me what you've got"
>> So it should the give you the stored procedure text as is.
>>
>> The only worry in programming terms is that 16000 bytes won't be enough to
>> hold what sql server gives us,
>> But think that highly unlikely, given we know what we're selecting...
>>
>> My guess would be that the users of defncopy could be counted on a couple
>> of hands.
>> The stripping off blanks behaviour would mess up many stored procedures
>> where a syntactically significant blank
>> fell at the end of one of the syscomments rows, e.g.
>> row 1: "....SELECT * FROM "
>> row 2: "MYTABLE WHERE....."
>>
>> so your fix is an improvement I have no doubt!
>> Make the change, and give it to Freddy as a patch.
>>
>> If this would then be your first contribution to the project, welcome!
>>
>> Bill
>>
>>
>> -----Original Message-----
>> From: FreeTDS [mailto:[email protected]] On Behalf Of Nem
>> W Schlecht
>> Sent: 14 June 2016 16:59
>> To: FreeTDS Development Group
>> Subject: Re: [freetds] Old dblib.c bug still there
>>
>> On Tue, Jun 14, 2016 at 7:43 AM, Thompson, William <
>> [email protected]
>> > wrote:
>>
>> > I Agree with Freddy - if you want to make a change to test it out, I'd
>> > make the change to defncopy.c Try changing that NTBSTRINGBIND to plain
>> > old STRINGBIND, and see how that affects the behaviour.
>> > It looks to me like it should stop the trailing blanks from being
>> > stripped off
>> >
>> >
>> >
>> Gave this a shot.  In my test stored procedure, it now adds around 12,000
>> extra spaces. Not quite what I was hoping for.  :)  Looks like it wants to
>> pad it out to 16,000 chars.
>>
>> I think it would work correctly if limited_dest_space weren't set in
>> dblib.c copy_data_to_host_var().
>>
>> Yes, I changed line 577 of defncopy.c from:
>>
>> erc = dbbind(dbproc, ctext, NTBSTRINGBIND, sizeof(sql_text), (BYTE *)
>> sql_text);
>>
>> to:
>>
>> erc = dbbind(dbproc, ctext, STRINGBIND, 0, (BYTE *) sql_text);
>>
>>
>> And now defncopy is working correctly on my test stored procedure.  Is
>> this going to mess anything else up, though?
>>
>>
>>
>>
>> >
>> > -----Original Message-----
>> > From: FreeTDS [mailto:[email protected]] On Behalf Of
>> > Frediano Ziglio
>> > Sent: 14 June 2016 12:58
>> > To: FreeTDS Development Group
>> > Subject: Re: [freetds] Old dblib.c bug still there
>> >
>> > 2016-06-12 18:59 GMT+01:00 Nem W Schlecht <[email protected]>:
>> > > Hello all,
>> > >
>> > > First off, I just want to thank all of the developers in this
>> > > community for continuing work on such a useful and excellent
>> > > application.  I cannot thank and appreciate you all enough!  Thank
>> > > you,
>> > Thank You, THANK YOU!!
>> > >
>> > >
>> > > I recently grabbed the last stable release and came across an issue
>> > > that seems to constantly be plaguing me and has been around for a
>> > > long time.  I had reported this back in July of 2014, but the
>> > > proposed solution (stripping all but one space) would still screw up
>> > > some of my stored procedures (that have lots of spaces in strings
>> > > for formatting
>> > purposes).
>> > >
>> > > My original post and reply:
>> > >   http://lists.ibiblio.org/pipermail/freetds/2014q3/028964.html
>> > >   http://lists.ibiblio.org/pipermail/freetds/2014q3/028966.html
>> > >
>> > > The issue is with lines 7311-7314 in dblib.c:
>> > >
>> > >    7311                 case NTBSTRINGBIND: /* strip trailing blanks,
>> > null
>> > > term */
>> > >    7312                     while (srclen && src[srclen - 1] == ' ') {
>> > >    7313                         --srclen;
>> > >    7314                     }
>> > >
>> > > Looks like something good to do, right?  Get rid of any blanks that
>> > > we don't need.  However, this keeps messing up my output when I use
>> > "defncopy"
>> > > to dump out some of my stored procedures.
>> > >
>> >
>> > Well... this is the expected behaviour of NTBSTRINGBIND. The problem
>> > is not in dblib.c but in defncopy that expects a different behaviour.
>> >
>> > > If it just so happens that you have a large stored procedure or
>> > > large string in that stored procedure *and* there are one or more
>> > > spaces exactly at a multiple of byte 4000, dblib.c will strip out
>> > > the spaces, causing either data corruption or an SQL syntax error in
>> > > the stored
>> > procedure.
>> > >
>> > > I'm attaching an example to reproduce the issue.
>> > >
>> > > Load in 'dbo.big_proc-load.sql' and then immediately run 'defncopy'
>> > > to dump it back out.  Each digit should have 40 spaces between it,
>> > > but on line 96, it only contains 8 spaces.  dblib grabbed the first
>> > > 4000 bytes, saw there there were spaces, and truncated it back to
>> > > the last
>> > non-space character.
>> > >
>> > > I've actually had this happen (just by pure chance) in actual code
>> > > as well, where a line like this:
>> > >
>> > >      SELECT some_field_name AS some_field_alias
>> > >
>> > > has been turned into:
>> > >
>> > >      SELECT some_field_nameAS some_field_alias
>> > >
>> > > Thus causing an error the next time the stored procedure is run,
>> > > since the field name is now incorrect.
>> > >
>> > > I've modified my copy of dblib.c and commented out lines 7312-7314
>> > > (as I did so 2 years ago and I've had to negative effects), but
>> > > would like to hear thoughts/opinions of the developers and would
>> > > really like this issue fixed.
>> > >
>> > > Again, thank you all so much for all of your hard work on FreeTDS!
>> > >
>> >
>> > Seems that all bindings with varlen != 0 (as you don't want an
>> > overflow
>> > too):
>> > - does blank padding;
>> > - are limited;
>> > - strip blanks.
>> > All stuff you don't want for your usage. The possible solution is to
>> > not use binding but dbdata and family.
>> >
>> > Frediano
>> > _______________________________________________
>> > FreeTDS mailing list
>> > [email protected]
>> > http://lists.ibiblio.org/mailman/listinfo/freetds
>> >
>> > ----------------------------------------------------------------------
>> > This message, and any attachments, is for the intended recipient(s)
>> > only, may contain information that is privileged, confidential and/or
>> > proprietary and subject to important terms and conditions available at
>> > http://www.bankofamerica.com/emaildisclaimer.   If you are not the
>> > intended recipient, please delete this message.
>> > _______________________________________________
>> > FreeTDS mailing list
>> > [email protected]
>> > http://lists.ibiblio.org/mailman/listinfo/freetds
>> >
>>
>>
>>
>> --
>> Nem W Schlecht <http://www.emptec.com/>
>>  "Perl did the magic.  I just waved the wand."
>> _______________________________________________
>> FreeTDS mailing list
>> [email protected]
>> http://lists.ibiblio.org/mailman/listinfo/freetds
>>
>> ----------------------------------------------------------------------
>> This message, and any attachments, is for the intended recipient(s) only,
>> may contain information that is privileged, confidential and/or proprietary
>> and subject to important terms and conditions available at
>> http://www.bankofamerica.com/emaildisclaimer.   If you are not the
>> intended recipient, please delete this message.
>> _______________________________________________
>> FreeTDS mailing list
>> [email protected]
>> http://lists.ibiblio.org/mailman/listinfo/freetds
>>
>
>
>
> --
> Nem W Schlecht                   [email protected]
> Empyreal Technologies    http://www.emptec.com/
>  "Perl did the magic.  I just waved the wand."
>
> _______________________________________________
> FreeTDS mailing list
> [email protected]
> http://lists.ibiblio.org/mailman/listinfo/freetds
>