Thread: Patch for ODBC driver (look for odbc.ini in more than one location)

Patch for ODBC driver (look for odbc.ini in more than one location)

From
"Ross Thomas"
Date:
Since most ODBC drivers and driver managers look for odbc.ini in several
locations on UNIX systems, it would seem sensible to have the PostgreSQL
driver do the same thing. (This idea occurred to me after five hours of
trying to figure out what libpsqlodbc didn't like about my /etc/odbc.ini,
but that's another story.)

iODBC, for example, does the following:

1) Checks $ODBCINI
2) Checks $HOME/.odbc.ini
3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
4) Checks system-wide odbc.ini (in /etc by default)

This is essentially backwards-compatible with the old behaviour, unless
$ODBCINI is defined (in which case, arguably, the DBA knows what he's doing
anyway...). It resembles the Windows approach of allowing both user and
system data sources.

The attached patch implements this search path in 7.1.3's
GetPrivateProfileString(). It patches the following files:

configure.in
src/Makefile.global.in
src/interfaces/odbc/GNUmakefile
src/interfaces/odbc/dlg_specific.h
src/interfaces/odbc/gpps.c

It works for me (GNU/Linux i686). Possible issues include:

- Security. Since this library could (unwisely) be called from a setuid
binary, this is very important. How sensible is it to incorporate the
contents of an environment variable directly into the first argument to
fopen()? All relevant calls to fopen() are read-only (PG_BINARY_R).

- Portability. The new code shouldn't be any less portable than the 7.1.3
code, but I might have missed something.

- Consistency. Is the configure script option (--with-odbc=DIR) properly
named? I followed the convention set by --with-odbcinst, but the name I
chose could conceivably be confused with --enable-odbc. It might be more
appropriate (albeit less consistent) to call it --with-odbc-ini.

- Should one obtain the current user's home directory from /etc/passwd by
doing getpwuid(getuid()) or getpwuid(geteuid())? Currently I - like the
7.1.3 version of GetPrivateProfileString() - use the former, but it's
something worth thinking about.

- While I think I understand the somewhat convoluted code I replaced in
GetPrivateProfileString(), I am occasionally wrong. ;)

Feedback on these and any other issues would be very much appreciated.

Regards,
Ross Thomas


Attachment

Re: Patch for ODBC driver (look for odbc.ini in more than one

From
Igor Kovalenko
Date:
I ran into this issue too. Not sure whether it is useful to check 4
locations, but current code is certainly broken. It does check for
odbcinst.ini in ~user and default location, but only ~user for odbc.ini.
First, that is inconsistent, then it is not very useful when you try to
use ODBC from something like PHP, which often will run as 'nobody'
(which often means no homedir).
Default location certainly should be searched and that would not require
big changes. Looking at the code I actually think original coder meant
to do that but overlooked that code will only work for odbcinst.ini.

As for security implications, since code aready does check ~user, I
would not think that adding default place would make it any less secure.

I don't really get it why do you want to add another configure option at
all, it is confusing enough already. I would think that odbcinst.ini and
[default] odbc.ini normally should be in the same place.

regards,
- igor

Ross Thomas wrote:
>
> Since most ODBC drivers and driver managers look for odbc.ini in several
> locations on UNIX systems, it would seem sensible to have the PostgreSQL
> driver do the same thing. (This idea occurred to me after five hours of
> trying to figure out what libpsqlodbc didn't like about my /etc/odbc.ini,
> but that's another story.)
>
> iODBC, for example, does the following:
>
> 1) Checks $ODBCINI
> 2) Checks $HOME/.odbc.ini
> 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> 4) Checks system-wide odbc.ini (in /etc by default)
>
> This is essentially backwards-compatible with the old behaviour, unless
> $ODBCINI is defined (in which case, arguably, the DBA knows what he's doing
> anyway...). It resembles the Windows approach of allowing both user and
> system data sources.
>
> The attached patch implements this search path in 7.1.3's
> GetPrivateProfileString(). It patches the following files:
>
> configure.in
> src/Makefile.global.in
> src/interfaces/odbc/GNUmakefile
> src/interfaces/odbc/dlg_specific.h
> src/interfaces/odbc/gpps.c
>
> It works for me (GNU/Linux i686). Possible issues include:
>
> - Security. Since this library could (unwisely) be called from a setuid
> binary, this is very important. How sensible is it to incorporate the
> contents of an environment variable directly into the first argument to
> fopen()? All relevant calls to fopen() are read-only (PG_BINARY_R).
>
> - Portability. The new code shouldn't be any less portable than the 7.1.3
> code, but I might have missed something.
>
> - Consistency. Is the configure script option (--with-odbc=DIR) properly
> named? I followed the convention set by --with-odbcinst, but the name I
> chose could conceivably be confused with --enable-odbc. It might be more
> appropriate (albeit less consistent) to call it --with-odbc-ini.
>
> - Should one obtain the current user's home directory from /etc/passwd by
> doing getpwuid(getuid()) or getpwuid(geteuid())? Currently I - like the
> 7.1.3 version of GetPrivateProfileString() - use the former, but it's
> something worth thinking about.
>
> - While I think I understand the somewhat convoluted code I replaced in
> GetPrivateProfileString(), I am occasionally wrong. ;)
>
> Feedback on these and any other issues would be very much appreciated.
>
> Regards,
> Ross Thomas
>
>   ------------------------------------------------------------------------
>                        Name: ini-search-patch
>    ini-search-patch    Type: unspecified type (application/octet-stream)
>                    Encoding: quoted-printable
>
>   ------------------------------------------------------------------------
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

Re: Patch for ODBC driver (look for odbc.ini in more than one

From
Hiroshi Inoue
Date:
Ross Thomas wrote:
>
> Since most ODBC drivers and driver managers look for odbc.ini in several
> locations on UNIX systems, it would seem sensible to have the PostgreSQL
> driver do the same thing. (This idea occurred to me after five hours of
> trying to figure out what libpsqlodbc didn't like about my /etc/odbc.ini,
> but that's another story.)
>
> iODBC, for example, does the following:
>
> 1) Checks $ODBCINI
> 2) Checks $HOME/.odbc.ini
> 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> 4) Checks system-wide odbc.ini (in /etc by default)

IIRC iODBC 3.0.5 has iodbcinst library which knows
where to write/get the profile values.
PostgreSQL 7.2 provides a configure option --with-iodbc
with which PostgreSQL doesn't use gpps.c at all but links
with the installed odbcinst library.

regards,
Hiroshi Inoue

Re: Patch for ODBC driver (look for odbc.ini in more than one location)

From
"Ross Thomas"
Date:
> Ross Thomas wrote:
> >
> > Since most ODBC drivers and driver managers look for odbc.ini in several
> > locations on UNIX systems, it would seem sensible to have the PostgreSQL
> > driver do the same thing. (This idea occurred to me after five hours of
> > trying to figure out what libpsqlodbc didn't like about my
/etc/odbc.ini,
> > but that's another story.)
> >
> > iODBC, for example, does the following:
> >
> > 1) Checks $ODBCINI
> > 2) Checks $HOME/.odbc.ini
> > 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> > 4) Checks system-wide odbc.ini (in /etc by default)


Hiroshi Inoue wrote:
>
> IIRC iODBC 3.0.5 has iodbcinst library which knows
> where to write/get the profile values.
> PostgreSQL 7.2 provides a configure option --with-iodbc
> with which PostgreSQL doesn't use gpps.c at all but links
> with the installed odbcinst library.

I did a quick scan of the PostgreSQL ChangeLog before starting the patch,
but must've missed that. It certainly sounds like a good idea.

But it also seems like a good idea for libpsqlodbc to check sensible
locations even when used with an earlier version of iODBC (or some other
driver manager).

Regards,
Ross Thomas




Re: Patch for ODBC driver (look for odbc.ini in more than

From
Peter Eisentraut
Date:
Igor Kovalenko writes:

> I ran into this issue too. Not sure whether it is useful to check 4
> locations, but current code is certainly broken. It does check for
> odbcinst.ini in ~user and default location, but only ~user for odbc.ini.
[...]
> Default location certainly should be searched and that would not require
> big changes. Looking at the code I actually think original coder meant
> to do that but overlooked that code will only work for odbcinst.ini.

That would seem to be a useful addition.  You could probably just use the
same directory that is configured for odbcinst.ini now.

> I don't really get it why do you want to add another configure option at
> all, it is confusing enough already. I would think that odbcinst.ini and
> [default] odbc.ini normally should be in the same place.

Linking against the appropriate libraries of the driver manager ensures
that the driver and the driver manager always look into the same place.

--
Peter Eisentraut   peter_e@gmx.net


Re: Patch for ODBC driver (look for odbc.ini in more thanone

From
Igor Kovalenko
Date:
Peter Eisentraut wrote:
>
> Igor Kovalenko writes:
>
> > I don't really get it why do you want to add another configure option at
> > all, it is confusing enough already. I would think that odbcinst.ini and
> > [default] odbc.ini normally should be in the same place.
>
> Linking against the appropriate libraries of the driver manager ensures
> that the driver and the driver manager always look into the same place.

There is no driver manager with psqlODBC driver (supplied with
postgres). UnixODBC and iODBC have a manager, but the supplied driver is
'stand-alone'.

- igor

Re: Patch for ODBC driver (look for odbc.ini in more than

From
Bruce Momjian
Date:
It looks like we already have this capability in 7.2:

/*
 * theIniFileName is searched for in:
 *  $HOME/theIniFileName
 *  theIniFileName
 *  ODBCINSTDIR/ODBCINST_INI
 */
DWORD
GetPrivateProfileString(const char *theSection, /* section name */
                        const char *theKey,     /* search key name */
                        const char *theDefault, /* default value if not
                                                 * found */
                        char *theReturnBuffer,  /* return value stored
                                                 * here */
                        size_t theReturnBufferLength,   /* byte length of return
                                                         * buffer */
                        const char *theIniFileName)     /* pathname of ini file
                                                         * to search */

---------------------------------------------------------------------------

> Since most ODBC drivers and driver managers look for odbc.ini in several
> locations on UNIX systems, it would seem sensible to have the PostgreSQL
> driver do the same thing. (This idea occurred to me after five hours of
> trying to figure out what libpsqlodbc didn't like about my /etc/odbc.ini,
> but that's another story.)
>
> iODBC, for example, does the following:
>
> 1) Checks $ODBCINI
> 2) Checks $HOME/.odbc.ini
> 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> 4) Checks system-wide odbc.ini (in /etc by default)
>
> This is essentially backwards-compatible with the old behaviour, unless
> $ODBCINI is defined (in which case, arguably, the DBA knows what he's doing
> anyway...). It resembles the Windows approach of allowing both user and
> system data sources.
>
> The attached patch implements this search path in 7.1.3's
> GetPrivateProfileString(). It patches the following files:
>
> configure.in
> src/Makefile.global.in
> src/interfaces/odbc/GNUmakefile
> src/interfaces/odbc/dlg_specific.h
> src/interfaces/odbc/gpps.c
>
> It works for me (GNU/Linux i686). Possible issues include:
>
> - Security. Since this library could (unwisely) be called from a setuid
> binary, this is very important. How sensible is it to incorporate the
> contents of an environment variable directly into the first argument to
> fopen()? All relevant calls to fopen() are read-only (PG_BINARY_R).
>
> - Portability. The new code shouldn't be any less portable than the 7.1.3
> code, but I might have missed something.
>
> - Consistency. Is the configure script option (--with-odbc=DIR) properly
> named? I followed the convention set by --with-odbcinst, but the name I
> chose could conceivably be confused with --enable-odbc. It might be more
> appropriate (albeit less consistent) to call it --with-odbc-ini.
>
> - Should one obtain the current user's home directory from /etc/passwd by
> doing getpwuid(getuid()) or getpwuid(geteuid())? Currently I - like the
> 7.1.3 version of GetPrivateProfileString() - use the former, but it's
> something worth thinking about.
>
> - While I think I understand the somewhat convoluted code I replaced in
> GetPrivateProfileString(), I am occasionally wrong. ;)
>
> Feedback on these and any other issues would be very much appreciated.
>
> Regards,
> Ross Thomas
>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Patch for ODBC driver (look for odbc.ini in more than

From
"Hiroshi Inoue"
Date:
> -----Original Message-----
> From: Bruce Momjian
>
> It looks like we already have this capability in 7.2:
>
> /*
>  * theIniFileName is searched for in:
>  *  $HOME/theIniFileName
>  *  theIniFileName
>  *  ODBCINSTDIR/ODBCINST_INI
>  */
> DWORD
> GetPrivateProfileString(const char *theSection, /* section name */
>                         const char *theKey,     /* search key name */
>                         const char *theDefault, /* default value if not
>                                                  * found */
>                         char *theReturnBuffer,  /* return value stored
>                                                  * here */
>                         size_t theReturnBufferLength,   /* byte
> length of return
>                                                          * buffer */
>                         const char *theIniFileName)     /*
> pathname of ini file
>                                                          * to search */
>
> ------------------------------------------------------------------
> ---------
>
> > Since most ODBC drivers and driver managers look for odbc.ini in several
> > locations on UNIX systems, it would seem sensible to have the PostgreSQL
> > driver do the same thing. (This idea occurred to me after five hours of
> > trying to figure out what libpsqlodbc didn't like about my
> /etc/odbc.ini,
> > but that's another story.)
> >
> > iODBC, for example, does the following:
> >
> > 1) Checks $ODBCINI
> > 2) Checks $HOME/.odbc.ini
> > 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> > 4) Checks system-wide odbc.ini (in /etc by default)

AFAIK only 3) is implemented. However there's no problem
if there's no user.

regards,
Hiroshi Inoue


Re: Patch for ODBC driver (look for odbc.ini in more than

From
"Ross Thomas"
Date:
> It looks like we already have this capability in 7.2:
>
> /*
>  * theIniFileName is searched for in:
>  *  $HOME/theIniFileName
>  *  theIniFileName
>  *  ODBCINSTDIR/ODBCINST_INI
>  */

Unless I'm missing something, this still doesn't match what "most" ODBC
drivers do - for example, it doesn't look in /etc (or other
./configure-defined location) for a system-wide file, or in $ODBCINI. This
can be annoying for a sysadmin wishing to provide a predefined set of data
sources.

Someone mentioned that 7.2 can use an iODBC library to read odbc.ini, which
is an improvement, but IMHO PostgreSQL's driver should be more flexible by
default, without having to link with a library. (This would also be useful
as far as binary packages go, as there needn't be a different PostgreSQL
driver package for each driver manager.)

Regards,
Ross Thomas




Re: Patch for ODBC driver (look for odbc.ini in more than

From
Bruce Momjian
Date:
> > > Since most ODBC drivers and driver managers look for odbc.ini in several
> > > locations on UNIX systems, it would seem sensible to have the PostgreSQL
> > > driver do the same thing. (This idea occurred to me after five hours of
> > > trying to figure out what libpsqlodbc didn't like about my
> > /etc/odbc.ini,
> > > but that's another story.)
> > >
> > > iODBC, for example, does the following:
> > >
> > > 1) Checks $ODBCINI
> > > 2) Checks $HOME/.odbc.ini
> > > 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> > > 4) Checks system-wide odbc.ini (in /etc by default)
>
> AFAIK only 3) is implemented. However there's no problem
> if there's no user.

Added to TODO:

  * Add config file check for $ODBCINI, $HOME/.odbc.ini, and /etc/odbc.ini

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Patch for ODBC driver (look for odbc.ini in more than

From
Bruce Momjian
Date:
Ross Thomas wrote:
> > It looks like we already have this capability in 7.2:
> >
> > /*
> >  * theIniFileName is searched for in:
> >  *  $HOME/theIniFileName
> >  *  theIniFileName
> >  *  ODBCINSTDIR/ODBCINST_INI
> >  */
>
> Unless I'm missing something, this still doesn't match what "most" ODBC
> drivers do - for example, it doesn't look in /etc (or other
> ./configure-defined location) for a system-wide file, or in $ODBCINI. This
> can be annoying for a sysadmin wishing to provide a predefined set of data
> sources.
>
> Someone mentioned that 7.2 can use an iODBC library to read odbc.ini, which
> is an improvement, but IMHO PostgreSQL's driver should be more flexible by
> default, without having to link with a library. (This would also be useful
> as far as binary packages go, as there needn't be a different PostgreSQL
> driver package for each driver manager.)

Agreed.  Can you supply a patch against current source so we can release
it with 7.3?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Patch for ODBC driver (look for odbc.ini in more than

From
Igor Kovalenko
Date:
Bruce Momjian wrote:
>
> > > > Since most ODBC drivers and driver managers look for odbc.ini in several
> > > > locations on UNIX systems, it would seem sensible to have the PostgreSQL
> > > > driver do the same thing. (This idea occurred to me after five hours of
> > > > trying to figure out what libpsqlodbc didn't like about my
> > > /etc/odbc.ini,
> > > > but that's another story.)
> > > >
> > > > iODBC, for example, does the following:
> > > >
> > > > 1) Checks $ODBCINI
> > > > 2) Checks $HOME/.odbc.ini
> > > > 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> > > > 4) Checks system-wide odbc.ini (in /etc by default)
> >
> > AFAIK only 3) is implemented. However there's no problem
> > if there's no user.
>
> Added to TODO:
>
>   * Add config file check for $ODBCINI, $HOME/.odbc.ini, and /etc/odbc.ini

Why /etc/odbc.ini? Shouldn't it be installpath/etc/odbc.ini?

- igor

Re: Patch for ODBC driver (look for odbc.ini in more than

From
Bruce Momjian
Date:
Igor Kovalenko wrote:
> Bruce Momjian wrote:
> >
> > > > > Since most ODBC drivers and driver managers look for odbc.ini in several
> > > > > locations on UNIX systems, it would seem sensible to have the PostgreSQL
> > > > > driver do the same thing. (This idea occurred to me after five hours of
> > > > > trying to figure out what libpsqlodbc didn't like about my
> > > > /etc/odbc.ini,
> > > > > but that's another story.)
> > > > >
> > > > > iODBC, for example, does the following:
> > > > >
> > > > > 1) Checks $ODBCINI
> > > > > 2) Checks $HOME/.odbc.ini
> > > > > 3) Checks ~/.odbc.ini (where ~ is obtained from struct passwd)
> > > > > 4) Checks system-wide odbc.ini (in /etc by default)
> > >
> > > AFAIK only 3) is implemented. However there's no problem
> > > if there's no user.
> >
> > Added to TODO:
> >
> >   * Add config file check for $ODBCINI, $HOME/.odbc.ini, and /etc/odbc.ini
>
> Why /etc/odbc.ini? Shouldn't it be installpath/etc/odbc.ini?

TODO updated:

 * Add config file check for $ODBCINI, $HOME/.odbc.ini, installpath/etc/odbc.ini

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Patch for ODBC driver (look for odbc.ini in more than

From
"Ross Thomas"
Date:
Bruce Momjian wrote:

> Ross Thomas wrote:
> > > It looks like we already have this capability in 7.2:
> > >
> > > /*
> > >  * theIniFileName is searched for in:
> > >  *  $HOME/theIniFileName
> > >  *  theIniFileName
> > >  *  ODBCINSTDIR/ODBCINST_INI
> > >  */
> >
> > Unless I'm missing something, this still doesn't match what "most" ODBC
> > drivers do - for example, it doesn't look in /etc (or other
> > ./configure-defined location) for a system-wide file, or in $ODBCINI.
This
> > can be annoying for a sysadmin wishing to provide a predefined set of
data
> > sources.
> >
> > Someone mentioned that 7.2 can use an iODBC library to read odbc.ini,
which
> > is an improvement, but IMHO PostgreSQL's driver should be more flexible
by
> > default, without having to link with a library. (This would also be
useful
> > as far as binary packages go, as there needn't be a different PostgreSQL
> > driver package for each driver manager.)
>
> Agreed.  Can you supply a patch against current source so we can release
> it with 7.3?

Sure thing.

Regards,
Ross Thomas