7.1 odbc bug & patch - Mailing list pgsql-patches

Your name               : Dave Bodenstab
Your email address      : imdave@mcs.net


System Configuration
---------------------
  Architecture (example: Intel Pentium)         : (shouldn't matter) Intel x86

  Operating System (example: Linux 2.0.26 ELF)  : (shouldn't matter) FreeBSD

  PostgreSQL version (example: PostgreSQL-7.1):   PostgreSQL-7.1

  Compiler used (example:  gcc 2.8.0)           : (shouldn't matter) gcc 2.7.2.1



Please enter a FULL description of your problem:
------------------------------------------------
I installed postgres 7.1 with --enable-odbc.  I then installed
tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
(http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz).  I could not
get either to work... postgres would not find the global odbcinst.ini
file.  I traced this to src/interfaces/odbc/gpps.c -- here are the
many things I think are wrong:

 40  GetPrivateProfileString(char *theSection,        /* section name */
  :
 50  {
 51      char        buf[MAXPGPATH];
 52      char       *ptr = 0;
  :
 63      int            j = 0;

 64      j = strlen(theIniFileName) + 1;
 65      ptr = (char *) getpwuid(getuid());    /* get user info */

 66      if (ptr == NULL)
 67      {
 68          if (MAXPGPATH - 1 < j)
 69              theIniFileName[MAXPGPATH - 1] = '\0';

 70          sprintf(buf, "%s", theIniFileName);
 71      }
 72      ptr = ((struct passwd *) ptr)->pw_dir;        /* get user home dir */
 73      if (ptr == NULL || *ptr == '\0')
 74          ptr = "/home";

 75      /*
 76       * This doesn't make it so we find an ini file but allows normal
 77       * processing to continue further on down. The likelihood is that the
 78       * file won't be found and thus the default value will be returned.
 79       */
 80      if (MAXPGPATH - 1 < strlen(ptr) + j)
 81      {
 82          if (MAXPGPATH - 1 < strlen(ptr))
 83              ptr[MAXPGPATH - 1] = '\0';
 84          else
 85              theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
 86      }

 87      sprintf(buf, "%s/%s", ptr, theIniFileName);

 88      /*
 89       * This code makes it so that a file in the users home dir overrides a
 90       * the "default" file as passed in
 91       */
 92      aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
 93      if (!aFile)
 94      {
 95          sprintf(buf, "%s", theIniFileName);
 96          aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
 97      }

+ Line 65 sets ptr, line 66 checks for NULL but line 72 dereferences
  ptr anyway.  Then line 73 checks for NULL again!?
+ Line 87 unconditionally ignores what line 70 did.
+ Lines 69, 83 and 85 modify the value passed to the function unbeknownst
  to the calling function.  The values could be a static string or might
  be further used by the calling function.
+ Truncating the strings might result in a string that accidently matches
  another, totally non-related file.
+ Line 96 checks ``buf?'', but buf[] is an array and the test will *always*
  be true.
+ The globally installed odbcinst.ini file is never checked.  This is
  why I couldn't get things to work -- it was looking for odbcinst.ini
  only in my home directory or the current directory.



Please describe a way to repeat the problem.   Please try to provide a
concise reproducible example, if at all possible:
----------------------------------------------------------------------
Run tclodbc and do a ``database db <DSNname>'' where ``DSNname'' is
one of the DSN's in /usr/local/etc/odbcinst.ini (or wherever the
global ini file is installed.)  The result is always the error
message that ``one of server,port,database,etc. are missing''.

Run libiodbc-2.50.3/samples/odbctest <DSNname>.  The command fails
to connect to the database and just exits.



If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------
I couldn't find any documentation as to what the correct behavior should
be, and the comments aren't any clearer.  I changed the code to look for
the odbcinst.ini file in 1) $HOME, 2) the current directory and 3) the
globally installed file.  I also fixed all the problems I mentioned
above.  Here is the patch, it works for me.  Use it any way you want.

--- gpps.c    2001/06/06 02:53:05    7.1
+++ gpps.c    2001/06/06 17:23:39    7.1.1.3
@@ -35,6 +35,7 @@
 #include <string.h>
 #include "misc.h"
 #include "gpps.h"
+#include "dlg_specific.h"

 #ifndef TRUE
 #define TRUE    ((BOOL)1)
@@ -44,6 +45,12 @@
 #endif


+/*
+ * theIniFileName is searched for in:
+ *     $HOME/theIniFileName
+ *     theIniFileName
+ *     ODBCINST_INI
+ */
 DWORD
 GetPrivateProfileString(char *theSection,        /* section name */
                         char *theKey,    /* search key name */
@@ -68,48 +75,36 @@
     size_t        aReturnLength = 0;
     BOOL        aSectionFound = FALSE;
     BOOL        aKeyFound = FALSE;
-    int            j = 0;

-    j = strlen(theIniFileName) + 1;
     ptr = (char *) getpwuid(getuid());    /* get user info */

-    if (ptr == NULL)
-    {
-        if (MAXPGPATH - 1 < j)
-            theIniFileName[MAXPGPATH - 1] = '\0';
-
-        sprintf(buf, "%s", theIniFileName);
-    }
-    ptr = ((struct passwd *) ptr)->pw_dir;        /* get user home dir */
-    if (ptr == NULL || *ptr == '\0')
+    if (ptr == NULL || (((struct passwd *) ptr)->pw_dir) == NULL || *(((struct passwd *) ptr)->pw_dir) == '\0')
         ptr = "/home";
+    else
+        ptr = ((struct passwd *) ptr)->pw_dir;        /* get user home dir */

     /*
-     * This doesn't make it so we find an ini file but allows normal
-     * processing to continue further on down. The likelihood is that the
-     * file won't be found and thus the default value will be returned.
+     * If it can't be opened because the paths are too long, then
+     * skip it, don't just truncate the path string...  The truncated path
+     * might accidently be an existing file.  The default value will be
+     * returned instead.
      */
-    if (MAXPGPATH - 1 < strlen(ptr) + j)
+    if (MAXPGPATH - 1 >= strlen(ptr) + 1 + strlen(theIniFileName))
     {
-        if (MAXPGPATH - 1 < strlen(ptr))
-            ptr[MAXPGPATH - 1] = '\0';
-        else
-            theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
+        sprintf(buf, "%s/%s", ptr, theIniFileName);
+        aFile = (FILE *) fopen(buf, PG_BINARY_R);
     }

-    sprintf(buf, "%s/%s", ptr, theIniFileName);
-
     /*
      * This code makes it so that a file in the users home dir overrides a
      * the "default" file as passed in
      */
-    aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
     if (!aFile)
     {
-        sprintf(buf, "%s", theIniFileName);
-        aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
+        aFile = (FILE *) fopen(theIniFileName, PG_BINARY_R);
+        if (!aFile)
+            aFile = (FILE *) fopen(ODBCINST_INI, PG_BINARY_R);
     }
-

     aLength = (theDefault == NULL) ? 0 : strlen(theDefault);