[R-sig-DB] [PATCH] segfault in RSQLite 0.5-4

[Seth Falcon]

Hi Ashish,

"Ashish Kulkarni" <ashish.kulkarni using kalyptorisk.com> writes:
> I encountered a segfault in RSQLite 0.5-4, it was a null pointer
> dereference which was introduced in r246. Apparently, under certain
> conditions sqlite3_column_decltype() returns NULL, which wasn't
> checked for at all and caused SQLite_decltype_to_type() to
> segfault. I've attached the patch against 0.5-4.

Thanks for the report.  I just returned from vacation which is why I
was not able to respond more quickly.  I can reproduce the crash and
will have a fix available by the end of the week.

As for you patch:

diff -ur RSQLite/src/RS-SQLite.c RSQLite/src/RS-SQLite.c
--- RSQLite/src/RS-SQLite.c	2007-04-25 22:13:13.000000000 +0530
+++ RSQLite/src/RS-SQLite.c	2007-07-05 19:00:20.444167700 +0530
@@ -903,8 +903,13 @@
     col_type = sqlite3_column_type(db_statement, j);
     if (col_type == SQLITE_NULL) {
         /* try to get type from origin column */
-        col_decltype = sqlite3_column_decltype(db_statement, j);
-        col_type = SQLite_decltype_to_type(col_decltype);
+        col_decltype = sqlite3_column_decltype(db_statement, j);

Not sure if it will come through in your MUA, but you've used DOS
style line endings which makes it hard for me to apply.

+        
+        /* if SQLite doesn't give the information, assume it is an integer */

I'm a bit surprised by this choice.  Can you explain why integer is
the desired default?  I was expecting to see SQLITE_TEXT here -- if
you don't know what a column in a result set is, you can always put it
into a character vector in R, you cannot always convert it to
integer.  Or am I missing something?

Does the following patch fix the crash for you?

diff --git a/SQLite/RSQLite/src/RS-SQLite.c b/SQLite/RSQLite/src/RS-SQLite.c
index 0545211..27e5b47 100644
--- a/SQLite/RSQLite/src/RS-SQLite.c
+++ b/SQLite/RSQLite/src/RS-SQLite.c
@@ -349,6 +349,8 @@ RS_SQLite_closeConnection(Con_Handle *conHandle)
 int SQLite_decltype_to_type(const char* decltype)
 {
     unsigned int h = 0;
+    if (!decltype)
+        return SQLITE_TEXT;
     int len = strlen(decltype);
     const unsigned char *zIn = (unsigned char*)decltype;
     const unsigned char *zEnd = (unsigned char*)&(decltype[len]);

The problem is that whenever a result set has a column with a NULL in
the first row, the type will be forced to text.  I can think of ways
of fixing this, but the solutions add complexity and will likely hurt
performance.  I wonder if anyone has a suggestion here?  One idea is
to provide a mechanism for users to specify the desired types of the
columns in a resultset.

+ seth

-- 
Seth Falcon | Computational Biology | Fred Hutchinson Cancer Research Center
http://bioconductor.org