Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.Data.Sqlite: Check error code when binding parameters #32613

Merged
merged 3 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ private IEnumerable<sqlite3_stmt> GetStatements()
? PrepareAndEnumerateStatements()
: _preparedStatements)
{
var boundParams = _parameters?.Bind(stmt) ?? 0;
var boundParams = _parameters?.Bind(stmt, Connection!.Handle!) ?? 0;

if (expectedParams != boundParams)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.Sqlite.Core/SqliteParameter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public virtual void ResetSqliteType()
_sqliteType = null;
}

internal bool Bind(sqlite3_stmt stmt)
internal bool Bind(sqlite3_stmt stmt, sqlite3 handle)
{
if (string.IsNullOrEmpty(ParameterName))
{
Expand All @@ -222,7 +222,7 @@ internal bool Bind(sqlite3_stmt stmt)
throw new InvalidOperationException(Resources.RequiresSet(nameof(Value)));
}

new SqliteParameterBinder(stmt, index, _value, _size, _sqliteType).Bind();
new SqliteParameterBinder(stmt, handle, index, _value, _size, _sqliteType).Bind();

return true;
}
Expand Down
31 changes: 25 additions & 6 deletions src/Microsoft.Data.Sqlite.Core/SqliteParameterBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ namespace Microsoft.Data.Sqlite
internal class SqliteParameterBinder : SqliteValueBinder
{
private readonly sqlite3_stmt _stmt;
private readonly sqlite3 _handle;
private readonly int _index;
private readonly int? _size;

public SqliteParameterBinder(sqlite3_stmt stmt, int index, object value, int? size, SqliteType? sqliteType)
public SqliteParameterBinder(sqlite3_stmt stmt, sqlite3 handle, int index, object value, int? size, SqliteType? sqliteType)
: base(value, sqliteType)
{
_stmt = stmt;
_handle = handle;
_index = index;
_size = size;
}
Expand All @@ -30,26 +32,43 @@ protected override void BindBlob(byte[] value)
Array.Copy(value, blob, _size.Value);
}

sqlite3_bind_blob(_stmt, _index, blob);
var rc = sqlite3_bind_blob(_stmt, _index, blob);
SqliteException.ThrowExceptionForRC(rc, _handle);
}

protected override void BindDoubleCore(double value)
=> sqlite3_bind_double(_stmt, _index, value);
{
var rc = sqlite3_bind_double(_stmt, _index, value);

SqliteException.ThrowExceptionForRC(rc, _handle);
}

protected override void BindInt64(long value)
=> sqlite3_bind_int64(_stmt, _index, value);
{
var rc = sqlite3_bind_int64(_stmt, _index, value);

SqliteException.ThrowExceptionForRC(rc, _handle);
}

protected override void BindNull()
=> sqlite3_bind_null(_stmt, _index);
{
var rc = sqlite3_bind_null(_stmt, _index);

SqliteException.ThrowExceptionForRC(rc, _handle);
}

protected override void BindText(string value)
=> sqlite3_bind_text(
{
var rc = sqlite3_bind_text(
_stmt,
_index,
ShouldTruncate(value.Length)
? value.Substring(0, _size!.Value)
: value);

SqliteException.ThrowExceptionForRC(rc, _handle);
}

private bool ShouldTruncate(int length)
=> _size.HasValue
&& length > _size.Value
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.Sqlite.Core/SqliteParameterCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,12 @@ protected override void SetParameter(int index, DbParameter value)
protected override void SetParameter(string parameterName, DbParameter value)
=> SetParameter(IndexOfChecked(parameterName), value);

internal int Bind(sqlite3_stmt stmt)
internal int Bind(sqlite3_stmt stmt, sqlite3 handle)
{
var bound = 0;
foreach (var parameter in _parameters)
{
if (parameter.Bind(stmt))
if (parameter.Bind(stmt, handle))
{
bound++;
}
Expand Down
62 changes: 62 additions & 0 deletions test/Microsoft.Data.Sqlite.Tests/SqliteCommandTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,78 @@
using System.Data;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Data.Sqlite.Properties;
using SQLitePCL;
using Xunit;
using static SQLitePCL.raw;

namespace Microsoft.Data.Sqlite;

public class SqliteCommandTest
{
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task Correct_error_code_is_returned_when_parameter_is_too_long(bool async) // Issue #27597
{
using var connection = new SqliteConnection("Data Source=:memory:");

if (async)
{
await connection.OpenAsync();
}
else
{
connection.Open();
}

using var command = connection.CreateCommand();
command.CommandText = """
CREATE TABLE "Products" (
"Id" INTEGER NOT NULL CONSTRAINT "PK_Products" PRIMARY KEY AUTOINCREMENT,
"Name" TEXT NOT NULL
);
""";
_ = async ? await command.ExecuteNonQueryAsync() : command.ExecuteNonQuery();

var sqliteProvider = (ISQLite3Provider)typeof(raw)
.GetProperty("Provider", BindingFlags.Static | BindingFlags.NonPublic)!
.GetValue(null)!;

sqliteProvider.sqlite3_limit(connection.Handle!, 0, 10);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericsink The "sqlite3_limit" function isn't mapped by default, which is fine, but for this bug I need to call it. Is there a better way of getting hold of the "raw" functions rather than using Reflection like this?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by "isn't mapped by default".

sqlite3_limit is in my provider API and is exposed statically by the raw class.

As a sample, my test case for the call looks like this:

        [Fact]
        public void test_call_sqlite3_limit()
        {
            using (sqlite3 db = ugly.open(":memory:"))
            {
                var query = raw.sqlite3_limit(db, raw.SQLITE_LIMIT_LENGTH, -1);
                Assert.True(query >= 0);

                const int limit = 10240;
                var current = raw.sqlite3_limit(db, raw.SQLITE_LIMIT_LENGTH, limit);
                Assert.Equal(query, current);

                query = raw.sqlite3_limit(db, raw.SQLITE_LIMIT_LENGTH, -1);
                Assert.Equal(limit, query);
            }
        }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericsink Thanks! I totally missed the public static methods!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries. You folks are probably going through ... a transition. :-)

Tag me anytime. Tag me twice if I miss the first one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Eric! Appreciate it, as always.


command.CommandText = @"INSERT INTO ""Products"" (""Name"") VALUES (@p0);";
command.Parameters.Add("@p0", SqliteType.Text);
command.Parameters[0].Value = new string('A', 15);
Batteries_V2.Init();
try
{
_ = async ? await command.ExecuteReaderAsync() : command.ExecuteReader();
}
catch (SqliteException ex)
{
Assert.Equal(18, ex.SqliteErrorCode);
}
finally
{
#if NET5_0_OR_GREATER
if (async)
{
await connection.CloseAsync();
}
else
{
connection.Close();
}
#else
connection.Close();
#endif
}
}

[Fact]
public void Ctor_sets_values()
{
Expand Down
Loading