Skip to content

Commit

Permalink
Handling explicit null bindings in SQLite
Browse files Browse the repository at this point in the history
These were being implicitly coerced into a string before reaching the `SqliteDatabase::Query::bind` function.

I've also added tests to confirm that you can inject BLOBs by binding a `Uint8Array`

Update src/workerd/api/sql.c++

Co-authored-by: Kenton Varda <kenton@cloudflare.com>
  • Loading branch information
geelen and kentonv committed May 23, 2023
1 parent 562fea4 commit 17c893f
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
22 changes: 20 additions & 2 deletions src/workerd/api/sql-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ async function test(storage) {
assert.equal(resultStr[0]["'hello'"], "hello");

// Test blob results
const resultBlob = [...sql.exec("SELECT x'ff'")];
const resultBlob = [...sql.exec("SELECT x'ff' as blob")];
assert.equal(resultBlob.length, 1);
const blob = new Uint8Array(resultBlob[0]["x'ff'"]);
const blob = new Uint8Array(resultBlob[0].blob);
assert.equal(blob.length, 1);
assert.equal(blob[0], 255);

Expand Down Expand Up @@ -453,6 +453,24 @@ async function test(storage) {
gc();
}
}

{
// Test binding blobs & nulls
sql.exec(`CREATE TABLE test_blob (id INTEGER PRIMARY KEY, data BLOB);`)
sql.prepare(
`INSERT INTO test_blob(data) VALUES(?),(ZEROBLOB(10)),(null),(?);`
)(
crypto.getRandomValues(new Uint8Array(12)), null
)
const results = Array.from(sql.exec(`SELECT * FROM test_blob`))
assert.equal(results.length, 4)
assert.equal(results[0].data instanceof ArrayBuffer, true)
assert.equal(results[0].data.byteLength, 12)
assert.equal(results[1].data instanceof ArrayBuffer, true)
assert.equal(results[1].data.byteLength, 10)
assert.equal(results[2].data, null)
assert.equal(results[3].data, null)
}
}

export class DurableObjectExample {
Expand Down
22 changes: 13 additions & 9 deletions src/workerd/api/sql.c++
Original file line number Diff line number Diff line change
Expand Up @@ -197,16 +197,20 @@ SqlStorage::Statement::Statement(SqliteDatabase::Statement&& statement)
kj::Array<const SqliteDatabase::Query::ValuePtr> SqlStorage::Cursor::mapBindings(
kj::ArrayPtr<BindingValue> values) {
return KJ_MAP(value, values) -> SqliteDatabase::Query::ValuePtr {
KJ_SWITCH_ONEOF(value) {
KJ_CASE_ONEOF(data, kj::Array<const byte>) {
return data.asPtr();
}
KJ_CASE_ONEOF(text, kj::String) {
return text.asPtr();
}
KJ_CASE_ONEOF(d, double) {
return d;
KJ_IF_MAYBE(v, value) {
KJ_SWITCH_ONEOF(*v) {
KJ_CASE_ONEOF(data, kj::Array<const byte>) {
return data.asPtr();
}
KJ_CASE_ONEOF(text, kj::String) {
return text.asPtr();
}
KJ_CASE_ONEOF(d, double) {
return d;
}
}
} else {
return nullptr;
}
KJ_UNREACHABLE;
};
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/sql.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class SqlStorage final: public jsg::Object, private SqliteDatabase::Regulator {
SqlStorage(SqliteDatabase& sqlite, jsg::Ref<DurableObjectStorage> storage);
~SqlStorage();

using BindingValue = kj::OneOf<kj::Array<const byte>, kj::String, double>;
using BindingValue = kj::Maybe<kj::OneOf<kj::Array<const byte>, kj::String, double>>;

class Cursor;
class Statement;
Expand Down

0 comments on commit 17c893f

Please # to comment.