Wrong filename handling in sqlite3_load_extension() for Cygwin

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Wrong filename handling in sqlite3_load_extension() for Cygwin

Jan Nijtmans
The function sqlite3_load_extension() on Cygwin expects a
win32 path, even though it is compiled for Cygwin.
How to reproduce? First create some extension, e.g.:
    gcc -shared -o wholenumber.dll ext/misc/wholenumber.c

Then compile the following little program:
================== main.c ================
#include <sqlite3.h>
#include <stdio.h>
void main(int argc, char **argv){
  sqlite3 *db;
  int rc;
  char *zErrMsg = 0;
  printf("loading extension: %s\n", argv[1]);
  sqlite3_initialize();
  sqlite3_open("foo.db", &db);
  sqlite3_enable_load_extension(db, 1);
  rc = sqlite3_load_extension(db, argv[1], "sqlite3_wholenumber_init",
&zErrMsg);
  if( rc!=SQLITE_OK ){
    fprintf(stderr, "Error: %s\n", zErrMsg);
    sqlite3_free(zErrMsg);
    rc = 1;
  }
}
============================================

    gcc main.c sqlite3.c
(This produces an executable "a.exe")

Then try:
    $ ./a.exe wholenumber.dll
    loading extension: wholenumber.dll
This works, but:
    $ ./a.exe $PWD/wholenumber.dll
    loading extension: /home/nijtmaj/wholenumber.dll
    Error: The specified module could not be found.
Hey, this is exactly where the dll is.
    $ ./a.exe `cygpath -w /home/nijtmaj/`wholenumber.dll
    loading extension: C:\Localdata\cygwin\home\nijtmaj\wholenumber.dll
When using win32 path, it works! That's not correct!

Suggested patch which fixes this is below. Note
that adding 17 lines to the function
winConvertFromUtf8Filename() means that 38 lines
can be removed from the function winGetTempname():
If we teach winConvertFromUtf8Filename() how to
handle cygwin paths, winGetTempname() doesn't
have to do this conversion any more, just deligate it
to winConvertFromUtf8Filename(). The total
amalgamation becomes 21 lines shorter.

If the function cygwin_conv_path() fails for whatever
reason (which should never happen!), the original path
translation is used as fall-back. This simplifies greatly
the error-handling: if there is something wrong with
the path, everything is cleaned up correctly and
handled exactly as before.

I hope this contribution is still in time for SQLite 3.8.4,
and given some time for proper evaluation.

Regards,
        Jan Nijtmans
==========================================
Index: src/os_win.c
==================================================================
--- src/os_win.c
+++ src/os_win.c
@@ -4131,10 +4131,27 @@
 ** function.
 */
 static void *winConvertFromUtf8Filename(const char *zFilename){
   void *zConverted = 0;
   if( osIsNT() ){
+#ifdef __CYGWIN__
+    if( !(winIsDriveLetterAndColon(zFilename)
+        && winIsDirSep(zFilename[2])) ){
+      int nByte = cygwin_conv_path(CCP_POSIX_TO_WIN_W, zFilename, 0, 0);
+      if( nByte>0 ){
+        zConverted = sqlite3MallocZero(nByte);
+        if ( zConverted==0 ){
+          return zConverted;
+        }
+        if( cygwin_conv_path(CCP_POSIX_TO_WIN_W, zFilename,
+                             zConverted, nByte)==0 ){
+          return zConverted;
+        }
+        sqlite3_free(zConverted);
+      }
+    }
+#endif
     zConverted = winUtf8ToUnicode(zFilename);
   }
 #ifdef SQLITE_WIN32_HAS_ANSI
   else{
     zConverted = sqlite3_win32_utf8_to_mbcs(zFilename);
@@ -4243,11 +4260,11 @@
       /* If the path starts with a drive letter followed by the colon
       ** character, assume it is already a native Win32 path; otherwise,
       ** it must be converted to a native Win32 path via the Cygwin API
       ** prior to using it.
       */
-      if( winIsDriveLetterAndColon(zDir) ){
+      if( 1 ){
         zConverted = winConvertFromUtf8Filename(zDir);
         if( !zConverted ){
           sqlite3_free(zBuf);
           OSTRACE(("TEMP-FILENAME rc=SQLITE_IOERR_NOMEM\n"));
           return SQLITE_IOERR_NOMEM;
@@ -4256,10 +4273,11 @@
           sqlite3_snprintf(nMax, zBuf, "%s", zDir);
           sqlite3_free(zConverted);
           break;
         }
         sqlite3_free(zConverted);
+#if 0 /* not necessary any more */
       }else{
         zConverted = sqlite3MallocZero( nMax+1 );
         if( !zConverted ){
           sqlite3_free(zBuf);
           OSTRACE(("TEMP-FILENAME rc=SQLITE_IOERR_NOMEM\n"));
@@ -4290,10 +4308,11 @@
           sqlite3_free(zUtf8);
           sqlite3_free(zConverted);
           break;
         }
         sqlite3_free(zConverted);
+#endif /* not necessary any more */
       }
     }
   }
 #elif !SQLITE_OS_WINRT && !defined(__CYGWIN__)
   else if( osIsNT() ){
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Wrong filename handling in sqlite3_load_extension() for Cygwin

Jan Nijtmans
2014-02-20 14:39 GMT+01:00 Jan Nijtmans <[hidden email]>:
> The function sqlite3_load_extension() on Cygwin expects a
> win32 path, even though it is compiled for Cygwin.

Here is a new version of my patch which fixes this bug,
which handles better the case that the path does not
contain slashes (just a bare filename). And added comments
to the function winConvertFromUtf8Filename(), making
it clearer what this function is supposed to do in Cygwin 1.7+

I plan to include some standard extensions in the
SQLite 3.8.4 build for Cygwin, this patch is needed
to make dynamical loading work at all on Cygwin.

I hope this patch will be given some time for proper
evaluation, so it can be included in SQLite 3.8.4.

The first chunk in this patch does the actual work,
the last 3 are only optimization, showing how other
code can be simplified when winConvertFromUtf8Filename
is modified to be able to handle Cygwin paths. More
optimizations like this are possible, and will be submitted
as follow-up.

Thanks to Joe for providing feedback at my previous
attempt! I hope this patch makes things clearer.

Regards,
       Jan Nijtmans

Index: src/os_win.c
==================================================================
--- src/os_win.c
+++ src/os_win.c
@@ -4127,14 +4127,51 @@
 /*
 ** Convert a UTF-8 filename into whatever form the underlying
 ** operating system wants filenames in.  Space to hold the result
 ** is obtained from malloc and must be freed by the calling
 ** function.
+**
+** On Cygwin 1.7 and higher, 3 possible input forms are accepted:
+** - If the filename starts with "<drive>:/" or "<drive>:\",
+**   it is converted to UTF-16 as-is.
+** - If the filename contains '/', it is assumed to be a
+**   Cygwin absolute path, it is converted to a win32
+**   absolute path in UTF-16.
+** - Otherwise it must be a filename only, the win32 filename
+**   is returned in UTF-16.
+** Note: The function cygwin_conv_path does not exist in
+**   Cygwin 1.5. Cygwin 1.7 does not run in Windows 95/98/ME.
+**   Therefore the !osIsNT() case does not need special handling.
+** Note 2: If the function cygwin_conv_path() fails, only
+**   UTF-8 -> UTF-16 conversion will be done. This can only
+**   happen when the file path >32k, in which case winUtf8ToUnicode()
+**   will fail too.
 */
 static void *winConvertFromUtf8Filename(const char *zFilename){
   void *zConverted = 0;
   if( osIsNT() ){
+#ifdef __CYGWIN__
+    if( !(winIsDriveLetterAndColon(zFilename)
+        && winIsDirSep(zFilename[2])) ){
+      int nByte;
+      int convertflag = CCP_POSIX_TO_WIN_W;
+      if( !strchr(zFilename, '/') ) convertflag |= CCP_RELATIVE;
+      nByte = cygwin_conv_path(convertflag,
+          zFilename, 0, 0);
+      if( nByte>0 ){
+        zConverted = sqlite3MallocZero(nByte);
+        if ( zConverted==0 ){
+          return zConverted;
+        }
+        if( cygwin_conv_path(convertflag, zFilename,
+                             zConverted, nByte)==0 ){
+          return zConverted;
+        }
+        sqlite3_free(zConverted);
+      }
+    }
+#endif
     zConverted = winUtf8ToUnicode(zFilename);
   }
 #ifdef SQLITE_WIN32_HAS_ANSI
   else{
     zConverted = sqlite3_win32_utf8_to_mbcs(zFilename);
@@ -4243,11 +4280,11 @@
       /* If the path starts with a drive letter followed by the colon
       ** character, assume it is already a native Win32 path; otherwise,
       ** it must be converted to a native Win32 path via the Cygwin API
       ** prior to using it.
       */
-      if( winIsDriveLetterAndColon(zDir) ){
+      if( 1 ){
         zConverted = winConvertFromUtf8Filename(zDir);
         if( !zConverted ){
           sqlite3_free(zBuf);
           OSTRACE(("TEMP-FILENAME rc=SQLITE_IOERR_NOMEM\n"));
           return SQLITE_IOERR_NOMEM;
@@ -4256,10 +4293,11 @@
           sqlite3_snprintf(nMax, zBuf, "%s", zDir);
           sqlite3_free(zConverted);
           break;
         }
         sqlite3_free(zConverted);
+#if 0 /* not necessary any more */
       }else{
         zConverted = sqlite3MallocZero( nMax+1 );
         if( !zConverted ){
           sqlite3_free(zBuf);
           OSTRACE(("TEMP-FILENAME rc=SQLITE_IOERR_NOMEM\n"));
@@ -4290,10 +4328,11 @@
           sqlite3_free(zUtf8);
           sqlite3_free(zConverted);
           break;
         }
         sqlite3_free(zConverted);
+#endif /* not necessary any more */
       }
     }
   }
 #elif !SQLITE_OS_WINRT && !defined(__CYGWIN__)
   else if( osIsNT() ){
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users