Empty query causes loop

[joriszwart]

By mistake stupid me executed an empty query. This resulted in an (infinite?) loop and hanged my application.

The minimal example below reproduces this. This example uses an in-memory database, but it occurs with a database file as well.

Version: v1.14.7

db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
    panic(err)
}
defer db.Close()

stmt, err := db.Prepare("") // empty query
if err != nil {
    panic(err)
}
defer stmt.Close()

rows, err := stmt.Query()
if err != nil {
    panic(err)
}
defer rows.Close()

count := 0
for rows.Next() {
    var dummy string
    rows.Scan(&dummy) // this can be omitted

    log.Printf("%d: [%+v]\n", count, dummy)
    count++
}

Results in:

0: []
1: []
2: []
3: []
4: []
...

I don't know what the expected behavior should be. Maybe return an error?

[rittneje]

The source of the bug is that sqlite3_prepare_v2 produces a null *sqlite3_stmt with no error when you do this. But this library does not check for that. https://github.com/mattn/go-sqlite3/blob/d33341ff0d20a3b46047dba17ad897b40705c563/sqlite3.go#L1749-L1752

Compounding the issue, each call to sqlite3_step will fail with SQLITE_MISUSE, but it gets dropped because of this: https://github.com/mattn/go-sqlite3/blob/d33341ff0d20a3b46047dba17ad897b40705c563/sqlite3.go#L2105-L2111

Basically, sqlite3_reset always vacuously succeeds when given a null statement, so we end up returning null from each call to Next(). Thus the loop never terminates.

In short, there are two bugs in play:

1. If sqlite3_prepare_v2 yields SQLITE_OK with a null prepared statement it should be treated as an error.
2. If sqlite3_reset yields SQLITE_OK after sqlite3_step fails we should still return an error. (Note: We can't just call lastError() in a situation like this since it might not be set properly. Maybe just convert to an ErrNo?)

[dolmen]

3900dc318797f0c829deb42ed9d723a30e3f60b0 has since been applied.

[joriszwart]

I can still reproduce this using v1.14.17 (latest tag). #963 was about panic'ing when specifying an empty string.

[ganigeorgiev]

@rittneje Shouldn't simply returning io.EOF instead of nil in the no rows nextSyncLocked check resolve the issue?

Edit: I've opened https://github.com/mattn/go-sqlite3/pull/1285 with the above suggestion and the tests seem to pass.

[charlievieth]

Considering that there a few bugs in this library that are due to sqlite3_prepare_v2 not being used properly (https://github.com/mattn/go-sqlite3/issues/1161 comes to mind) and that there might be existing user programs that rely on said broken behavior has there been any discussion of creating a v2 branch that resolves these issues without maintaining backwards compatibility for programs that rely on the current (and sometimes broken) behavior?