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

Started by Ross Thomasover 24 years ago14 messagespatches
Jump to latest
#1Ross Thomas
ross@grinfinity.com

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

Attachments:

ini-search-patchapplication/octet-stream; name=ini-search-patchDownload+83-47
#2Igor Kovalenko
Igor.Kovalenko@motorola.com
In reply to: Ross Thomas (#1)
Re: Patch for ODBC driver (look for odbc.ini in more than one

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:

Show quoted text

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

#3Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Ross Thomas (#1)
Re: Patch for ODBC driver (look for odbc.ini in more than one

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

#4Ross Thomas
ross@grinfinity.com
In reply to: Ross Thomas (#1)
Re: Patch for ODBC driver (look for odbc.ini in more than one location)

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

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Igor Kovalenko (#2)
Re: Patch for ODBC driver (look for odbc.ini in more than

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

#6Igor Kovalenko
Igor.Kovalenko@motorola.com
In reply to: Peter Eisentraut (#5)
Re: Patch for ODBC driver (look for odbc.ini in more thanone

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

#7Bruce Momjian
bruce@momjian.us
In reply to: Ross Thomas (#1)
Re: Patch for ODBC driver (look for odbc.ini in more than

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
#8Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Bruce Momjian (#7)
Re: Patch for ODBC driver (look for odbc.ini in more than

-----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

#9Ross Thomas
ross@grinfinity.com
In reply to: Bruce Momjian (#7)
Re: Patch for ODBC driver (look for odbc.ini in more than

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

#10Bruce Momjian
bruce@momjian.us
In reply to: Ross Thomas (#9)
Re: Patch for ODBC driver (look for odbc.ini in more than

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
#11Bruce Momjian
bruce@momjian.us
In reply to: Hiroshi Inoue (#8)
Re: Patch for ODBC driver (look for odbc.ini in more than

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
#12Igor Kovalenko
Igor.Kovalenko@motorola.com
In reply to: Bruce Momjian (#11)
Re: Patch for ODBC driver (look for odbc.ini in more than

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

#13Bruce Momjian
bruce@momjian.us
In reply to: Igor Kovalenko (#12)
Re: Patch for ODBC driver (look for odbc.ini in more than

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
#14Ross Thomas
ross@grinfinity.com
In reply to: Bruce Momjian (#10)
Re: Patch for ODBC driver (look for odbc.ini in more than

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