ericsink / SQLitePCL.raw

A Portable Class Library (PCL) for low-level (raw) access to SQLite
Apache License 2.0
512 stars 106 forks source link

`sqlite3_bind_text` calls with empty `Span<byte>` result in `null` values #557

Closed nil4 closed 7 months ago

nil4 commented 8 months ago

The method sqlite3_bind_text(sqlite3_stmt stmt, int index, ReadOnlySpan<byte> val), called with ReadOnlySpan.Empty when inserting into a text column, results in null values, instead of empty strings.

With reference to the SQLite docs, namely:

int sqlite3_bind_text(sqlite3_stmt*,int,const char*,int,void(*)(void*));

In those routines that have a fourth argument, its value is the number of bytes in the parameter. [...] If a non-negative fourth parameter is provided to sqlite3_bind_text() or sqlite3_bind_text16() or sqlite3_bind_text64() then that parameter must be the byte offset where the NUL terminator would occur assuming the string were NUL terminated.

The current behaviour seems surprising and unexpected. Passing an empty span with non-negative length, it seems reasonable to expect the same result as having a literal empty string ('') in the SQL text, i.e. an empty string to be bound, and not null.

A repro program is provided below. It inserts an empty string, once using a SQL literal string, and again calling sqlite3_bind_text with an empty span.

The literal value is stored and can be retrieved as expected. However, the bound empty span results in null being stored.

What version of SQLitePCLRaw are you using? If you are using one of the SQLitePCLRaw bundle packages, which one?

SQLitePCLRaw.bundle_green 2.1.6

What platform are you running on? What operating system? Which version? What CPU?

Reproduces on MacOS Sonoma (14.0) and Windows 11 (22H2, 10.0.22621.2283), both ARM64.

What target framework are you building for? Are you on .NET Framework or the newer stuff (.NET Core, .NET 5+, etc)?

.NET (Core) 8.0.100-rc.1.23455.8

Are you using the command line, or an IDE? Which IDE? Which version of that IDE?

Reproduces from the command line, as well as IDE (Rider), thus I assume IDE version may not be relevant.

Are you using PackageReference or packages.config?

PackageReference

Sometimes other packages using SQLitePCLRaw cause problems when they are mixed together. What other packages are you including in your project?

None.

How can we reproduce the problem you are seeing? Your issue will get attention much faster if you attach a minimal reproduction sample project.

In a .NET console app project, add a package reference to SQLitePCLRaw.bundle_green 2.1.6, e.g.:

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>net8.0</TargetFramework>
    </PropertyGroup>
    <ItemGroup>
        <PackageReference Include="SQLitePCLRaw.bundle_green" Version="2.1.6" />
    </ItemGroup>
</Project>

Replace Program.cs with the code below.

An assertion is triggered when SQLITE_NULL (5) is unexpectedly found after insert:

Process terminated. Assertion failed. Expected sqlite3_column_type = TEXT (3), got 5 at Program.VerifyTextInserted(sqlite3 db) in Program.cs:line 40 at Program.Main() in Program.cs:line 31

using System;
using System.Runtime.CompilerServices;
using SQLitePCL;
using static SQLitePCL.raw;
using static System.Diagnostics.Debug;

static class Program
{
    static void Main()
    {
        Batteries_V2.Init();
        using sqlite3 db = OpenInMemory();

        sqlite3_exec(db, "create table tab(fld text);").ShouldBe(SQLITE_OK);

        // insert via literal value
        sqlite3_exec(db, "insert into tab(fld) values('');").ShouldBe(SQLITE_OK);

        VerifyTextInserted(db);

        // clean up
        sqlite3_exec(db, "delete from tab;").ShouldBe(SQLITE_OK);

        // insert via parameter value
        using (sqlite3_stmt insert = Prepare(db, "insert into tab(fld) values (@v);"u8))
        {
            sqlite3_bind_text(insert, 1, ReadOnlySpan<byte>.Empty).ShouldBe(SQLITE_OK); // <-- NOTE: empty ROS 
            sqlite3_step(insert).ShouldBe(SQLITE_DONE);
        }

        VerifyTextInserted(db); // <-- finds SQLITE_NULL (5) instead of empty string
    }

    static void VerifyTextInserted(sqlite3 db)
    {
        using sqlite3_stmt query = Prepare(db, "select fld from tab limit 1;"u8);
        sqlite3_step(query).ShouldBe(SQLITE_ROW);

        int sqlType = sqlite3_column_type(query, 0);
        Assert(sqlType is SQLITE_TEXT, $"Expected sqlite3_column_type = TEXT ({SQLITE_TEXT}), got {sqlType}");

        ReadOnlySpan<byte> span = sqlite3_column_blob(query, 0);
        Assert(span.IsEmpty, $"Expected sqlite3_column_blob = empty span, got {span.Length:N0} bytes");

        sqlite3_step(query).ShouldBe(SQLITE_DONE);
    }

    static sqlite3_stmt Prepare(sqlite3 db, ReadOnlySpan<byte> sql)
    {
        sqlite3_prepare_v2(db, sql, out sqlite3_stmt result, out ReadOnlySpan<byte> tail).ShouldBe(SQLITE_OK);
        Assert(tail.IsEmpty);
        return result;
    }

    static sqlite3 OpenInMemory()
    {
        const int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_PRIVATECACHE | SQLITE_OPEN_MEMORY;
        sqlite3_open_v2(null, out sqlite3 result, flags, vfs: null).ShouldBe(SQLITE_OK);
        return result;
    }

    static void ShouldBe(this int rc, int expected, [CallerArgumentExpression(nameof(rc))] string? call = null)
    {
        if (rc != expected) Assert(false, $"Expected {call} to return {expected}, got {rc}");
    }
}
nil4 commented 8 months ago

The root cause appears similar to that described and handled for sqlite3_bind_blob at:

https://github.com/ericsink/SQLitePCL.raw/blob/9c66b4f618d9d6831e83c16a8036daea83df4ddb/src/providers/provider.tt#L1586-L1595

This was confirmed by testing the SQLite C API behavior with the program below.

C API test ```c #include #include #include #include #include #include "sqlite3.h" void verify_text_inserted(sqlite3 *db, const char* context); int main(int argc, char *argv[]) { printf("SQLite version: %s\n", sqlite3_libversion()); sqlite3 *db; int rc = sqlite3_open_v2(":memory:", &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_MEMORY, NULL); assert(rc == SQLITE_OK); char *errmsg; rc = sqlite3_exec(db, "create table tab(fld text);", NULL, NULL, &errmsg); assert(rc == SQLITE_OK && errmsg == NULL); // insert literal empty string rc = sqlite3_exec(db, "insert into tab(fld) values('');", NULL, NULL, &errmsg); assert(rc == SQLITE_OK && errmsg == NULL); verify_text_inserted(db, "Literal empty string"); // clean up rc = sqlite3_exec(db, "delete from tab;", NULL, NULL, &errmsg); assert(rc == SQLITE_OK && errmsg == NULL); // insert bound empty string param char *insert_params = "insert into tab(fld) values (@v);"; sqlite3_stmt *query; rc = sqlite3_prepare_v2(db, insert_params, strlen(insert_params), &query, NULL); assert(rc == SQLITE_OK); rc = sqlite3_bind_text(query, 1, "", 0, NULL); // <-- NOTE: zero-length explicitly passed assert(rc == SQLITE_OK); rc = sqlite3_step(query); assert(rc == SQLITE_DONE); sqlite3_finalize(query); verify_text_inserted(db, "Bound empty string"); sqlite3_close(db); } void verify_text_inserted(sqlite3 *db, const char *context) { char *sql = "select fld from tab limit 1;"; sqlite3_stmt *query; int rc = sqlite3_prepare_v2(db, sql, strlen(sql), &query, NULL); assert(rc == SQLITE_OK); rc = sqlite3_step(query); assert(rc == SQLITE_ROW); int sql_type = sqlite3_column_type(query, 0); if (sql_type != SQLITE_TEXT) printf("Expected to find SQLITE_TEXT (%d) but got %d", SQLITE_TEXT, sql_type); assert(sql_type == SQLITE_TEXT); int byte_count = sqlite3_column_bytes(query, 0); assert(byte_count == 0); const void *ptr = sqlite3_column_blob(query, 0); printf("%s: got sql_type=%d, byte_count=%d, ptr=%p\n", context, sql_type, byte_count, ptr); sqlite3_finalize(query); } ```

Running this produces expected results:

Literal empty string: got sql_type=3, byte_count=0, ptr=0x0 Bound empty string: got sql_type=3, byte_count=0, ptr=0x0

However, if line 40 is changed to pass in a NULL pointer, i.e.:

-     rc = sqlite3_bind_text(query, 1, "", 0, NULL);
+     rc = sqlite3_bind_text(query, 1, NULL, 0, NULL);

Then the behavior changes and SQLITE_NULL (5) is inserted:

Literal empty string: got sql_type=3, byte_count=0, ptr=0x0 Assertion failed: (sql_type == SQLITE_TEXT), function verify_text_inserted, file test.c, line 67. Expected to find SQLITE_TEXT (3) but got 5

This suggests that the issue reported here is caused by:

Pull request https://github.com/ericsink/SQLitePCL.raw/pull/558 suggests a potential fix for this corner case in the sqlite3_bind_text* functions.

ericsink commented 8 months ago

Hmmm. Thanks for the report. The behavior you've described does seem incorrect.

The code has been this way for years, so I wonder why the problem has not been noticed earlier. I need to look a bit more closely before I make the code change.

nil4 commented 8 months ago

The issue popped up on our radar when inserting UTF-8 byte spans (empty strings) into SQLite columns declared NOT NULL -- the unexpected conversions to SQLITE_NULL caused the statements to fail right away.

I can only guess that other (more common?) use cases might bind C# (UTF-16) strings or utf8z parameters instead, which could avoid this issue on those code paths. But in our case, ReadOnlySpan was required, and so we need to work around it.

I understand that a detailed review is needed; happy to help in any way I can to see the fix proposed in the PR (or something similar to it) go through.

ericsink commented 7 months ago

This was included in 2.1.7, which has been pushed up to nuget.