Skip to content

Commit

Permalink
Fix to prevent double marshaling of data.
Browse files Browse the repository at this point in the history
Moving data between JavaScript and native is extremely slow, especially for binary which Cordova has to base64 encode and then transmit over XHR or as an URL parameter. The last thing we want to do is send more data. For some reason SQL statements and their parameters were being sent twice because they were duplicated to a `query` member. That member was only used by the iOS plugin, so this fix removes it and updates the iOS plugin to use the sql and params members like the others.

Additionally, the iOS plugin is updated to handle unsupported types during binding and a test was added to verify.

(@brodybits) from pull #170, adding www/SQLitePlugin.js updated from SQLitePlugin.coffee.md
  • Loading branch information
aarononeal authored and Chris Brody committed Feb 6, 2015
1 parent 7fc7b76 commit e73cc61
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 17 deletions.
2 changes: 0 additions & 2 deletions SQLitePlugin.coffee.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,6 @@ License for common Javascript: MIT or Apache
tropts.push
qid: qid
# for ios version:
query: [request.sql].concat(request.params)
sql: request.sql
params: request.params
Expand Down
34 changes: 20 additions & 14 deletions src/ios/SQLitePlugin.m
Original file line number Diff line number Diff line change
Expand Up @@ -352,24 +352,24 @@ -(void) executeSql: (CDVInvokedUrlCommand*)command
-(CDVPluginResult*) executeSqlWithDict: (NSMutableDictionary*)options andArgs: (NSMutableDictionary*)dbargs
{
NSString *dbPath = [self getDBPath:[dbargs objectForKey:@"dbname"]];

NSMutableArray *query_parts = [options objectForKey:@"query"];
NSString *query = [query_parts objectAtIndex:0];

if (dbPath == NULL) {
return [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"You must specify database path"];
}
if (query == NULL) {
return [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"You must specify a query to execute"];

NSString *sql = [options objectForKey:@"sql"];
if (sql == NULL) {
return [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"You must specify a sql query to execute"];
}

NSMutableArray *params = [options objectForKey:@"params"]; // optional

NSValue *dbPointer = [openDBs objectForKey:dbPath];
if (dbPointer == NULL) {
return [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"No such database, you must open it first"];
}
sqlite3 *db = [dbPointer pointerValue];

const char *sql_stmt = [query UTF8String];
const char *sql_stmt = [sql UTF8String];
NSDictionary *error = nil;
sqlite3_stmt *statement;
int result, i, column_type, count;
Expand All @@ -392,9 +392,9 @@ -(CDVPluginResult*) executeSqlWithDict: (NSMutableDictionary*)options andArgs: (
if (sqlite3_prepare_v2(db, sql_stmt, -1, &statement, NULL) != SQLITE_OK) {
error = [SQLitePlugin captureSQLiteErrorFromDb:db];
keepGoing = NO;
} else {
for (int b = 1; b < query_parts.count; b++) {
[self bindStatement:statement withArg:[query_parts objectAtIndex:b] atIndex:b];
} else if(params != NULL) {
for (int b = 0; b < params.count; b++) {
[self bindStatement:statement withArg:[params objectAtIndex:b] atIndex:(b+1)];
}
}

Expand Down Expand Up @@ -489,13 +489,19 @@ -(void)bindStatement:(sqlite3_stmt *)statement withArg:(NSObject *)arg atIndex:(
} else if (strcmp(numberType, @encode(double)) == 0) {
sqlite3_bind_double(statement, argIndex, [numberArg doubleValue]);
} else {
sqlite3_bind_text(statement, argIndex, [[NSString stringWithFormat:@"%@", arg] UTF8String], -1, SQLITE_TRANSIENT);
sqlite3_bind_text(statement, argIndex, [[arg description] UTF8String], -1, SQLITE_TRANSIENT);
}
} else { // NSString
NSString *stringArg = (NSString *)arg;
NSString *stringArg;

if ([arg isKindOfClass:[NSString class]]) {
stringArg = (NSString *)arg;
} else {
stringArg = [arg description]; // convert to text
}

NSData *data = [stringArg dataUsingEncoding:NSUTF8StringEncoding];

sqlite3_bind_text(statement, argIndex, data.bytes, data.length, SQLITE_TRANSIENT);
sqlite3_bind_text(statement, argIndex, data.bytes, data.length, SQLITE_TRANSIENT);
}
}

Expand Down
34 changes: 34 additions & 0 deletions test-www/www/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,40 @@
});
});

// This test shows that the plugin does not throw an error when trying to serialize
// an unsupported parameter type. Blob becomes an empty dictionary on iOS, for example,
// and so this verifies the type is converted to a string and continues. Web SQL does
// the same but on the JavaScript side and converts to a string like `[object Blob]`.
if (typeof ArrayBuffer !== "undefined") test(suiteName + "unsupported parameter type as string", function() {
var db = openDatabase("Blob-test.db", "1.0", "Demo", DEFAULT_SIZE);
ok(!!db, "db object");
stop(1);

db.transaction(function(tx) {
ok(!!tx, "tx object");

var buffer = new ArrayBuffer(5);
var view = new Uint8Array(buffer);
view[0] = 'h'.charCodeAt();
view[1] = 'e'.charCodeAt();
view[2] = 'l'.charCodeAt();
view[3] = 'l'.charCodeAt();
view[4] = 'o'.charCodeAt();
var blob = new Blob([view.buffer], { type:"application/octet-stream" });

tx.executeSql('DROP TABLE IF EXISTS test_table');
tx.executeSql('CREATE TABLE IF NOT EXISTS test_table (foo blob)');
tx.executeSql('INSERT INTO test_table VALUES (?)', [blob], function(tx, res) {
ok(true, "insert as string succeeds");
start(1);
});
start(1);
}, function(err) {
ok(false, "transaction does not serialize real data but still should not fail: " + err.message);
start(2);
});
});

test(suiteName + "readTransaction should throw on modification", function() {
stop();
var db = openDatabase("Database-readonly", "1.0", "Demo", DEFAULT_SIZE);
Expand Down
1 change: 0 additions & 1 deletion www/SQLitePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@
};
tropts.push({
qid: qid,
query: [request.sql].concat(request.params),
sql: request.sql,
params: request.params
});
Expand Down

0 comments on commit e73cc61

Please sign in to comment.