FreeTDS Development Group

["Eric Deutsch" <edeutsch AT systemsbiology.org>]

Hi all, since it looks like my name was hacked out, I better speak up!
I just downloaded the latest from CVS, and I agree that this TEXT
truncation is a revived problem that was fixed and now broken again.  As
of convert.c:1.12, we had:

           memcpy(dest, src, cplen);
!          /* 2001-06-15 Deutsch changed [cplen-1] to [cplen] */
!          dest[cplen] = '\0';
           return strlen(dest);

but then as of 1.13 ("ODBC patch from Viktar Klimkovitch" by brianb)

           memcpy(dest, src, cplen);
!          dest[cplen-1] = '\0';
           return strlen(dest);

I'd left my calling card in the code with the hope that anyone who
wanted it different could contact me.  I would like very much to revert
to my fix and investigate why it was changed back.  My platform (Linux
x86 to SQL Server 2000 w/ DBD::Sybase on TDS 7.0) needs version A of the
code else the last character gets trunctated.  Who has a test case that
clearly needs option B (Viktar?)  Let's see if we can settle this.

Thanks!
Eric


> -----Original Message-----
> From: Lowden, James K [mailto:LowdenJK AT bernstein.com]
> Sent: Monday, July 08, 2002 12:23 PM
> To: TDS Development Group
> Subject: [freetds] RE: Losing last character of TEXT fields
> 
> > From: L J Bayuk [mailto:lbayuk AT mindspring.com]
> > Sent: July 7, 2002 9:30 PM
> >
> > When I select from a TEXT field, the last character of
> > the value is lost. (CHAR and VARCHAR are OK).  This only happens
when
> > using a recent FreeTDS "current" CVS snapshot, not when using
> > FreeTDS-0.53.  It works fine in 0.53.
> >
> > I think the problem is an off-by-1 error here in
> > src/tds/convert.c in the
> > function tds_convert_text():
> >          cplen = srclen > destlen ? destlen : srclen;
> >          memcpy(dest, src, cplen);
> > *        dest[cplen-1] = '\0';
> >          return strlen(dest);
> 
> Brian, what do think about this and the proposed patch?  The story in
cvs
> is
> an unhappy one.  I looked at a diff for src/tds/convert.c between
version
> 1.12 and 1.13; there's a little tug of war over whether and when cplen
> should be decremented.  We seem to be oscillating between truncating
the
> data and overflowing the buffer.  :/  My ten foot pole says not to
touch
> it.
> 
> 
> --jkl
> 
> > I think that (*) line should be: dest[cplen] = '\0';
> > (that's how it was at 0.53) but it seems to me this could
> > overflow the dest
> > buffer.  I browsed the CVS on SourceForge and found that this
> > was changed
> > in rev 1.13 of that file on May 25, 2002 by Victor K. but it
> > isn't clear
> > why. I would like to propose the patch below. It changes it back,
and
> > includes another change to prevent dest buffer overflow, and
> > a third change
> > in how to get the return value. These make sense to me, and
> > seem to work,
> > but I'm new with this so dissenting opinions would be welcome...
> >
> > *** convert.c.bak   Tue Jun 25 21:44:27 2002
> > --- convert.c       Sat Jul  6 10:41:43 2002
> > ***************
> > *** 122,131 ****
> >      switch(desttype) {
> >         case SYBTEXT:
> >         case SYBCHAR:
> > !    cplen = srclen > destlen ? destlen : srclen;
> >            memcpy(dest, src, cplen);
> > !          dest[cplen-1] = '\0';
> > !          return strlen(dest);
> >         case SYBBINARY:
> >      cplen = srclen > destlen ? destlen : srclen;
> >            memcpy(dest, src, cplen);
> > --- 122,131 ----
> >      switch(desttype) {
> >         case SYBTEXT:
> >         case SYBCHAR:
> > !    cplen = srclen >= destlen ? destlen-1 : srclen;
> >            memcpy(dest, src, cplen);
> > !          dest[cplen] = '\0';
> > !          return cplen;
> >         case SYBBINARY:
> >      cplen = srclen > destlen ? destlen : srclen;
> >            memcpy(dest, src, cplen);
> 
> ---
> You are currently subscribed to freetds as:
[edeutsch AT systemsbiology.org]
> To unsubscribe, forward this message to leave-freetds-
> 127899P AT franklin.oit.unc.edu