Bug #79190 for DBD-ODBC: Username and password are not added to the dsn string

Fri Aug 24 10:16:47 2012 cserna.zsolt [...] gmail.com - Ticket created

Subject:

Username and password are not added to the dsn string

Reproducing the error: DBI->connect("dbi:ODBC:DSN=myserver", "username", "password"); DBD-ODBC is expected to add the "username" and "password" strings to the DSN specified (resulting "DSN=myserver;UID=username;PWD=password"), but it's not adding them in the releases 1.37 and above (there might be other versions affected). The problem is in the dsnHasDriverOrDSN function in the dbdimp.c file as it returns false even if the dsn string starts with "DSN=". dbdimp.c:533: *cp++ = toupper(*cp); This increments the cp pointer first, then converts it to uppercase. This omits the first letter from the result string so the comparison to "DSN=" will be incorrect. I've attached my patch (against 1.39) to the issue which fixed the problem for me.

Subject:

dbdimp.c.diff

--- DBD-ODBC-1.39/dbdimp.c 2012-07-07 08:14:24.000000000 -0400 +++ DBD-ODBC-1.39.patched/dbdimp.c 2012-08-24 10:09:37.000000000 -0400 @@ -530,7 +530,8 @@ strncpy(upper_dsn, dsn, sizeof(upper_dsn)-1); upper_dsn[sizeof(upper_dsn)-1] = '\0'; while (*cp != '\0') { - *cp++ = toupper(*cp); + *cp = toupper(*cp); + cp++; } return (strncmp(upper_dsn, "DSN=", 4) == 0 || strncmp(upper_dsn, "DRIVER=", 7) == 0);

Mon Sep 03 08:38:12 2012 bohica [...] ntlworld.com - Correspondence added

On Fri Aug 24 10:16:47 2012, csernazs wrote: Sorry for the late reply but I was at YAPC::EU and then on holiday. Show quoted text

> > Reproducing the error: > > DBI->connect("dbi:ODBC:DSN=myserver", "username", "password"); > > > DBD-ODBC is expected to add the "username" and "password" strings to the > DSN specified (resulting "DSN=myserver;UID=username;PWD=password"), but > it's not adding them in the releases 1.37 and above (there might be > other versions affected).

It works for me: $ perl -MDBI -le 'my $h = DBI->connect("dbi:ODBC:DSN=baugi");' DBI connect('DSN=baugi','',...) failed: [unixODBC][Easysoft][SQL Server Driver][SQL Server]Login failed for user ''. (SQL-28000) at -e line 1. $ perl -MDBI -le 'my $h = DBI->connect("dbi:ODBC:DSN=baugi","sa","easysoft");' Show quoted text

> The problem is in the dsnHasDriverOrDSN function in the dbdimp.c file as > it returns false even if the dsn string starts with "DSN=". > > dbdimp.c:533: > *cp++ = toupper(*cp); > > This increments the cp pointer first, then converts it to uppercase. > This omits the first letter from the result string so the comparison to > "DSN=" will be incorrect.

It should not do that. ++ in this case is a post increment operator. I'd be interested to know what compiler you are using and on what operating system as this sounds like a bug in your compiler. Compile the following as see what it does: #include <stdio.h> #include <ctype.h> #include <string.h> main() { char f[20]; char *cp; strcpy(f, "abcdefg"); cp = f; while(*cp != '\0') { *cp++ = toupper(*cp); } printf("%s\n", f); } Compile with optimization. For me it outputs: ABCDEFG Show quoted text

> I've attached my patch (against 1.39) to the issue which fixed the > problem for me. >

Martin -- Martin J. Evans Wetherby, UK

Mon Sep 03 08:38:13 2012 The RT System itself - Status changed from 'new' to 'open'

Tue Sep 04 03:23:27 2012 cserna.zsolt [...] gmail.com - Correspondence added

From:

cserna.zsolt [...] gmail.com

First, thanks for looking into this issue. I compiled the example code with various versions of gcc and optimalizations. Here are my results (gcc version, optimalization, program output): 3.2 -O0 ABCDEFG 3.2 -O1 BCDEFG 3.2 -O2 BCDEFG 3.2 -O3 BCDEFG 3.4 -O0 ABCDEFG 3.4 -O1 BCDEFG 3.4 -O2 BCDEFG 3.4 -O3 BCDEFG 4.1 -O0 ABCDEFG 4.1 -O1 ABCDEFG 4.1 -O2 ABCDEFG 4.1 -O3 ABCDEFG 4.7.1 -O0 ABCDEFG 4.7.1 -O1 ABCDEFG 4.7.1 -O2 ABCDEFG 4.7.1 -O3 ABCDEFG We built the package with gcc 3.4 with optimization enabled (-O2 or -O3), and that resulted this behavior. We needed to use this version of gcc as it's the default on the system (RHEL 4), but I think we can compile the package with gcc 4.1 in the future - as it's pure C and it should be compatible. On Hétf 2012. Szept 03 08:38:12, MJEVANS wrote: Show quoted text

> On Fri Aug 24 10:16:47 2012, csernazs wrote: > > Sorry for the late reply but I was at YAPC::EU and then on holiday. >

> > > > Reproducing the error: > > > > DBI->connect("dbi:ODBC:DSN=myserver", "username", "password"); > > > > > > DBD-ODBC is expected to add the "username" and "password" strings to the > > DSN specified (resulting "DSN=myserver;UID=username;PWD=password"), but > > it's not adding them in the releases 1.37 and above (there might be > > other versions affected).

> > It works for me: > > $ perl -MDBI -le 'my $h = DBI->connect("dbi:ODBC:DSN=baugi");' > DBI connect('DSN=baugi','',...) failed: [unixODBC][Easysoft][SQL Server > Driver][SQL Server]Login failed for user ''. (SQL-28000) at -e line 1. > > $ perl -MDBI -le 'my $h = > DBI->connect("dbi:ODBC:DSN=baugi","sa","easysoft");' >

> > The problem is in the dsnHasDriverOrDSN function in the dbdimp.c file as > > it returns false even if the dsn string starts with "DSN=". > > > > dbdimp.c:533: > > *cp++ = toupper(*cp); > > > > This increments the cp pointer first, then converts it to uppercase. > > This omits the first letter from the result string so the comparison to > > "DSN=" will be incorrect.

> > It should not do that. ++ in this case is a post increment operator. I'd > be interested to know what compiler you are using and on what operating > system as this sounds like a bug in your compiler. Compile the following > as see what it does: > > #include <stdio.h> > #include <ctype.h> > #include <string.h> > > main() { > char f[20]; > char *cp; > > strcpy(f, "abcdefg"); > cp = f; > while(*cp != '\0') { > *cp++ = toupper(*cp); > } > printf("%s\n", f); > } > > Compile with optimization. For me it outputs: > > ABCDEFG >

> > I've attached my patch (against 1.39) to the issue which fixed the > > problem for me. > >

> > Martin

Tue Sep 04 10:05:16 2012 bohica [...] ntlworld.com - Correspondence added

On Tue Sep 04 03:23:27 2012, csernazs wrote: Show quoted text

> > First, thanks for looking into this issue. > > I compiled the example code with various versions of gcc and > optimalizations. > > Here are my results (gcc version, optimalization, program output): > > 3.2 -O0 ABCDEFG > 3.2 -O1 BCDEFG > 3.2 -O2 BCDEFG > 3.2 -O3 BCDEFG > 3.4 -O0 ABCDEFG > 3.4 -O1 BCDEFG > 3.4 -O2 BCDEFG > 3.4 -O3 BCDEFG > 4.1 -O0 ABCDEFG > 4.1 -O1 ABCDEFG > 4.1 -O2 ABCDEFG > 4.1 -O3 ABCDEFG > 4.7.1 -O0 ABCDEFG > 4.7.1 -O1 ABCDEFG > 4.7.1 -O2 ABCDEFG > 4.7.1 -O3 ABCDEFG > > > We built the package with gcc 3.4 with optimization enabled (-O2 or > -O3), and that resulted this behavior. We needed to use this version of > gcc as it's the default on the system (RHEL 4), but I think we can > compile the package with gcc 4.1 in the future - as it's pure C and it > should be compatible.

After further investigation it appears that line of code contains an undefined action as = is not a valid sequence point. In actual fact, if you compile with -Wall/-Wsequence-point and omit the -Oanything gcc will issue the warning: dbdimp.c:535:10: warning: operation on ‘cp’ may be undefined [-Wsequence-point] so I've changed it as you did. Thanks for the report. This fix will be in the next release. Martin -- Martin J. Evans Wetherby, UK

Wed Sep 05 05:00:58 2012 bohica [...] ntlworld.com - Status changed from 'open' to 'resolved'

Wed Sep 05 05:00:58 2012 bohica [...] ntlworld.com - Fixed in 1.40_1 added