Thread: Patch for ODBC driver (look for odbc.ini in more than one location)
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
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
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
> 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
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
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
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
> -----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
> 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
> > > 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
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
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
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
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