Fix http://github.com/orlandov/node-sqlite/issues#issue/5

There was an off by one bug in the way bindings for strings were happening. A
NULL literal was being appended to string bindings. Thanks to @carter-thaxton
for reporting it.

src/statement.cc

@@ -180,8 +180,8 @@ Handle<Value> Statement::BindObject(const Arguments& args) {
 
     // setting key type
     pairs->key_type = KEY_STRING;
-    char *key = (char *) calloc(1, keyValue.length() + 1);
-    strcpy(key, *keyValue);
+    char *key = (char *) calloc(1, keyValue.length()+1);
+    memcpy(key, *keyValue, keyValue.length()+1);
     pairs->key = key;
 
     // setup value
@@ -200,10 +200,10 @@ Handle<Value> Statement::BindObject(const Arguments& args) {
     else if (val->IsString()) {
       pairs->value_type = VALUE_STRING;
       String::Utf8Value text(val);
-      char *value = (char *) calloc(text.length()+1, sizeof(char*));
-      strcpy(value, *text);
+      char *value = (char *) calloc(text.length(), sizeof(char));
+      memcpy(value, *text, text.length());
       pairs->value = value;
-      pairs->value_size = text.length()+1;
+      pairs->value_size = text.length();
     }
     else if (val->IsNull() || val->IsUndefined()) {
       pairs->value_type = VALUE_NULL;
@@ -274,10 +274,10 @@ Handle<Value> Statement::BindArray(const Arguments& args) {
     else if (val->IsString()) {
       pairs->value_type = VALUE_STRING;
       String::Utf8Value text(val);
-      char *value = (char *) calloc(text.length()+1, sizeof(char*));
-      strcpy(value, *text);
+      char *value = (char *) calloc(text.length(), sizeof(char));
+      memcpy(value, *text, text.length());
       pairs->value = value;
-      pairs->value_size = text.length()+1;
+      pairs->value_size = text.length();
     }
     else if (val->IsNull() || val->IsUndefined()) {
       pairs->value_type = VALUE_NULL;
@@ -311,7 +311,7 @@ Handle<Value> Statement::BindArray(const Arguments& args) {
 // Bind single placeholder by name:
 //   statement.bind('$x', value, callback);
 //
-// Bind placeholder by index:
+// Bind placeholder by position:
 //   statement.bind(1, value, callback);
 
 Handle<Value> Statement::Bind(const Arguments& args) {
@@ -340,8 +340,8 @@ Handle<Value> Statement::Bind(const Arguments& args) {
     String::Utf8Value keyValue(args[0]);
     pair->key_type = KEY_STRING;
 
-    char *key = (char *) calloc(1, keyValue.length() + 1);
-    strcpy(key, *keyValue);
+    char *key = (char *) calloc(1, keyValue.length()+1);
+    memcpy(key, *keyValue, keyValue.length()+1);
 
     pair->key = key;
   }
@@ -370,10 +370,10 @@ Handle<Value> Statement::Bind(const Arguments& args) {
   else if (args[1]->IsString()) {
     pair->value_type = VALUE_STRING;
     String::Utf8Value text(args[1]);
-    char *value = (char *) calloc(text.length()+1, sizeof(char*));
-    strcpy(value, *text);
+    char *value = (char *) calloc(text.length(), sizeof(char));
+    memcpy(value, *text, text.length());
     pair->value = value;
-    pair->value_size = text.length()+1;
+    pair->value_size = text.length();
   }
   else if (args[1]->IsNull() || args[1]->IsUndefined()) {
     pair->value_type = VALUE_NULL;

tests/test-bind-placeholders.js

@@ -11,10 +11,10 @@ puts = sys.puts;
 inspect = sys.inspect;
 
 var name = "Binding statement place holders";
-var suite = exports[name] = new TestSuite("Binding statement place holders");
+var suite = exports[name] = new TestSuite(name);
 
 var tests = [
-  { "Bind placeholders by index":
+  { "Bind placeholders by position":
     function (assert, finished) {
       var self = this;
       self.db.open(":memory:", function () {

tests/test-bind-strings.js

+require.paths.push(__dirname + '/..');
+
+sys = require('sys');
+fs = require('fs');
+path = require('path');
+
+TestSuite = require('async-testing/async_testing').TestSuite;
+sqlite = require('sqlite');
+
+// puts = sys.puts;
+puts = function () {};
+
+inspect = sys.inspect;
+
+var name = "Test binding strings to placeholders";
+var suite = exports[name] = new TestSuite(name);
+
+function checkTable(db, assert, callback) {
+  db.prepare('SELECT * FROM tbl WHERE foo="hello"', function (error, stmt) {
+    puts("prepared2 ok", inspect(arguments));
+    assert.ok(!error);
+    stmt.step(function (error, row) {
+      puts("step2 ok", inspect(arguments));
+      assert.ok(!error);
+      assert.ok(row, "We should get a row back");
+      callback();
+    });
+  });
+}
+
+function setupTableWithBindArray(db, assert, callback) {
+  db.prepareAndStep("CREATE TABLE tbl (foo TEXT)", function (error, stmt) {
+    puts("created ok", inspect(arguments));
+    assert.ok(!error);
+    db.prepare("INSERT INTO tbl (foo) VALUES (?)", function (error, stmt) {
+      assert.ok(!error);
+      puts("prepared ok", inspect(arguments));
+      stmt.bindArray(["hello"], function (error) {
+        assert.ok(!error);
+        checkTableCreated(stmt, assert, callback);
+      });
+    });
+  });
+}
+
+function setupTableWithBindByKey(db, assert, callback) {
+  db.prepareAndStep("CREATE TABLE tbl (foo TEXT)", function (error, stmt) {
+    puts("created ok", inspect(arguments));
+    assert.ok(!error);
+    db.prepare("INSERT INTO tbl (foo) VALUES ($x)", function (error, stmt) {
+      assert.ok(!error);
+      puts("prepared ok", inspect(arguments));
+      stmt.bind('$x', "hello", function (error) {
+        assert.ok(!error);
+        checkTableCreated(stmt, assert, callback);
+      });
+    });
+  });
+}
+
+function setupTableWithBindByPosition(db, assert, callback) {
+  db.prepareAndStep("CREATE TABLE tbl (foo TEXT)", function (error, stmt) {
+    puts("created ok", inspect(arguments));
+    assert.ok(!error);
+    db.prepare("INSERT INTO tbl (foo) VALUES (?)", function (error, stmt) {
+      assert.ok(!error);
+      puts("prepared ok", inspect(arguments));
+      stmt.bind(1, "hello", function (error) {
+        assert.ok(!error);
+        checkTableCreated(stmt, assert, callback);
+      });
+    });
+  });
+}
+
+function checkTableCreated(stmt, assert, callback) {
+  puts("bound", inspect(arguments));
+  stmt.step(function (error, row) {
+    assert.ok(!error);
+    assert.ok(!row);
+    puts("stepped", inspect(arguments));
+    stmt.finalize(function () {
+      callback();
+    });
+  });
+}
+
+var tests = [
+  { "Check strings are bound ok using an array":
+    function (assert, finished) {
+      var self = this;
+      self.db.open(":memory:", function () {
+        setupTableWithBindArray(self.db, assert, function () {
+          checkTable(self.db, assert, function () {
+            finished();
+          });
+        });
+      });
+    }
+  }
+, { "Check strings are bound ok using keys":
+    function (assert, finished) {
+      var self = this;
+      self.db.open(":memory:", function () {
+        setupTableWithBindByKey(self.db, assert, function () {
+          checkTable(self.db, assert, function () {
+            finished();
+          });
+        });
+      });
+    }
+  }
+, { "Check strings are bound ok using placeholder position":
+    function (assert, finished) {
+      var self = this;
+      self.db.open(":memory:", function () {
+        setupTableWithBindByPosition(self.db, assert, function () {
+          checkTable(self.db, assert, function () {
+            finished();
+          });
+        });
+      });
+    }
+  }
+];
+
+for (var i=0,il=tests.length; i < il; i++) {
+  suite.addTests(tests[i]);
+}
+
+var currentTest = 0;
+var testCount = tests.length;
+
+suite.setup(function(finished, test) {
+  this.db = new sqlite.Database();
+  finished();
+});
+suite.teardown(function(finished) {
+  if (this.db) this.db.close(function (error) {
+                               finished();
+                             });
+  ++currentTest == testCount;
+});
+
+if (module == require.main) {
+  suite.runTests();
+}