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

NullReferenceException thrown when DbString instance is null #1425

Closed
LanceRomance opened this issue Mar 23, 2020 · 6 comments · Fixed by #2003
Closed

NullReferenceException thrown when DbString instance is null #1425

LanceRomance opened this issue Mar 23, 2020 · 6 comments · Fixed by #2003

Comments

@LanceRomance
Copy link

LanceRomance commented Mar 23, 2020

I found when calling a stored procedure that has a DbString property and that instance is null, a NullReference exception is thrown.

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at ParamInfo5dd7247d-fd49-42a6-8b6e-f45fb45c461e(IDbCommand , Object )
   at Dapper.CommandDefinition.SetupCommand(IDbConnection cnn, Action`2 paramReader)
   at Dapper.SqlMapper.ExecuteCommand(IDbConnection cnn, CommandDefinition& command, Action`2 paramReader)
   at Dapper.SqlMapper.ExecuteImpl(IDbConnection cnn, CommandDefinition& command)
   at Dapper.SqlMapper.Execute(IDbConnection cnn, String sql, Object param, IDbTransaction transaction, Nullable`1 commandTimeout, Nullable`1 commandType)
   at ConsoleApp1.Program.Main(String[] args)

Example:

using (var conn = new SqlConnection(connString))
{
	DbString nullDbString = null;
	var parameterObject = new
	{
		Value1 = nullDbString,
	};
	
	conn.Execute("Test_Insert", parameterObject, null, null, CommandType.StoredProcedure);
}

I bumped into this due to code that looked something like:

public class Model
{
	public string Value { get; set; }
}
public class Model2
{
	public Model Instance { get; set; }
}

public static class Extensions
{
	public static DbString ToDbString(this string value)
	{
		return new DbString() { Value = value };
	}
}
var model2 = new Model2();
model2.Instance = null; //null due to a missing model in a json payload or something

using (var conn = new SqlConnection(connString))
{
	var p = new
	{
		Value1 = model2?.Instance?.Value, //this is fine
		Value2 = model2?.Instance?.Value?.ToDbString() //this causes a null ref to be thrown
	};
	
	conn.Execute("Test_Insert", p, null, null, CommandType.StoredProcedure);
}

I would expect the same behavior as passing a null string - a conversion to DbNull.Value.

@TimThrill
Copy link

Do we have any plan to resolve this?

@sirphilliptubell
Copy link

Also having this issue

@sirphilliptubell
Copy link

sirphilliptubell commented Nov 21, 2023

I ran across this again using the latest version (v2.1.21) and found a fix (at least for my scenario).

Original code that had a problem:

var param = new {
   sqlParam = maybeNullStr is null ? null : new DbString() { Value = maybeNullStr, Length = maybeNullStr.Length }
};

What I was tempted to do - which gets around the null reference exception

var param = new {
   sqlParam = new DbString() { Value = maybeNullStr, Length = maybeNullStr?.Length ?? 0 }
};

However if you're working with an Always Encrypted column like I was, you need to set the default size to the actual size of the sql column, otherwise you'll get an error like Operand type clash: nvarchar(4000) encrypted with ... is incompatible with nvarchar(123)

var param = new {
   CardNumber = new DbString() { Value = maybeNullStr, Length = string.IsNullOrEmpty(maybeNullStr) ? 123 : maybeNullStr.Length }
};
eg: for a column like this: "CREATE TABLE CreditCards ([CardNumber] NVARCHAR(123) NULL)

@LanceRomance you should just remove the question mark in Value?.ToDbString()

(edited to work with null and empty strings)

@mgravell
Copy link
Member

I'll see if we can fix this in the lib, sorry.

I'm also looking to improve this in AOT: DapperLib/DapperAOT#89

@mgravell
Copy link
Member

mgravell commented Nov 23, 2023

Right, so I had a look at this, and I can't see the problem; can someone help me clarify the failing scenario? yes you do need to assign a non-null DbString for it to work correctly, but the value can be null, so:

var args = new { x = new DbString() { Value = maybeNullStr, ... } };

Re:

However if you're working with an Always Encrypted column like I was, you need to set the default size to the actual size of the sql column, otherwise you'll get an error like Operand type clash: nvarchar(4000) encrypted with ... is incompatible with nvarchar(123)

Right; so set the correct size? Isn't that meant to be constant here?

var args = new { x = new DbString() { Value = maybeNullStr, Size = 200 } };

where 200 is the size defined by the column

I have looked at adding a .ctor (locally, not pushed yet):

var args = new { x = new DbString(maybeNullStr, 200 } };

@mgravell
Copy link
Member

mgravell commented Nov 23, 2023

Trying to something sensible when the value is null is tricky, as we won't know how to configure the parameter; I'm tweaking things to at least be clearer - the following now passes locally:

        [Fact]
        public void DbStringNullHandling()
        {
            // without lengths
            var obj = new { x = new DbString("abc"), y = (DbString?)new DbString(null) };
            var row = connection.QuerySingle<(string? x,string? y)>("select @x as x, @y as y", obj);
            Assert.Equal("abc", row.x);
            Assert.Null(row.y);

            // with lengths
            obj = new { x = new DbString("abc", 200), y = (DbString?)new DbString(null, 200) };
            row = connection.QuerySingle<(string? x, string? y)>("select @x as x, @y as y", obj);
            Assert.Equal("abc", row.x);
            Assert.Null(row.y);

            // null raw value - give clear message, at least
            obj = obj with { y = null };
            var ex = Assert.Throws<InvalidOperationException>(() => connection.QuerySingle<(string? x, string? y)>("select @x as x, @y as y", obj));
            Assert.Equal("Member 'y' is an ICustomQueryParameter and cannot be null", ex.Message);
        }

The AOT work is probably the place to move any actual functionality enhancements; is there anything I'm still missing about the basic failure scenario here, that isn't addressed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants