Skip to content

Commit 80d0e2b

Browse files
committed
cgsqlite,.: fix regression with empty strings
A final step refactor in 81bdfd0 introduced a bug where empty strings became NULL in some code paths. This fixes the regression and introduces a test to prevent reoccurrence. Updates tailscale/corp#9199
1 parent 7ab00bd commit 80d0e2b

File tree

2 files changed

+63
-1
lines changed

2 files changed

+63
-1
lines changed

cgosqlite/cgosqlite.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,12 @@ import (
5757
"github.com/tailscale/sqlite/sqliteh"
5858
)
5959

60-
var emptyStrPtr = (*C.char)(unsafe.Pointer(unsafe.StringData("")))
60+
// emptyStrPtr is the char* pointer constant used when binding empty strings to
61+
// avoid the need to allocate new storage in each invocation.
62+
var (
63+
emptyStrBacking [1]byte
64+
emptyStrPtr = (*C.char)(unsafe.Pointer(&emptyStrBacking[0]))
65+
)
6166

6267
func init() {
6368
C.sqlite3_initialize()

sqlite_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,63 @@ func TestEmptyString(t *testing.T) {
335335
}
336336
}
337337

338+
// TestEmptyStringNotNull verifies that binding an empty string results in an
339+
// empty string value, not NULL. This is a regression test for a bug where
340+
// unsafe.StringData("") could return nil, causing sqlite3_bind_text64 to
341+
// receive a NULL pointer and treat the value as NULL instead of "".
342+
func TestEmptyStringNotNull(t *testing.T) {
343+
db := openTestDB(t)
344+
ctx := t.Context()
345+
346+
exec(t, db, "CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)")
347+
exec(t, db, "INSERT INTO t (id, val) VALUES (?, ?)", 1, "")
348+
exec(t, db, "INSERT INTO t (id, val) VALUES (?, ?)", 2, nil)
349+
exec(t, db, "INSERT INTO t (id, val) VALUES (?, ?)", 3, "ok")
350+
351+
var val sql.NullString
352+
if err := db.QueryRowContext(ctx, "SELECT val FROM t WHERE id = 1").Scan(&val); err != nil {
353+
t.Fatal(err)
354+
}
355+
if !val.Valid {
356+
t.Fatal("empty string was stored as NULL")
357+
}
358+
if val.String != "" {
359+
t.Fatalf("val=%q, want empty string", val.String)
360+
}
361+
362+
if err := db.QueryRowContext(ctx, "SELECT val FROM t WHERE id = 2").Scan(&val); err != nil {
363+
t.Fatal(err)
364+
}
365+
if val.Valid {
366+
t.Fatalf("NULL was stored as %q", val.String)
367+
}
368+
369+
var countEmpty, countNull int
370+
if err := db.QueryRowContext(ctx, "SELECT count(*) FROM t WHERE val = ''").Scan(&countEmpty); err != nil {
371+
t.Fatal(err)
372+
}
373+
if countEmpty != 1 {
374+
t.Fatalf("countEmpty=%d, want 1", countEmpty)
375+
}
376+
if err := db.QueryRowContext(ctx, "SELECT count(*) FROM t WHERE val IS NULL").Scan(&countNull); err != nil {
377+
t.Fatal(err)
378+
}
379+
if countNull != 1 {
380+
t.Fatalf("countNull=%d, want 1", countNull)
381+
}
382+
383+
var length sql.NullInt64
384+
if err := db.QueryRowContext(ctx, "SELECT length(val) FROM t WHERE id = 1").Scan(&length); err != nil {
385+
t.Fatal(err)
386+
}
387+
if !length.Valid {
388+
t.Fatal("length of empty string returned NULL")
389+
}
390+
if length.Int64 != 0 {
391+
t.Fatalf("length=%d, want 0", length.Int64)
392+
}
393+
}
394+
338395
func TestExecScript(t *testing.T) {
339396
db := openTestDB(t)
340397
conn, err := db.Conn(context.Background())

0 commit comments

Comments
 (0)