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

Fix derived parameters containing typename incorrectly #1020

Merged
merged 3 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -379,24 +379,33 @@ internal static Exception StreamClosed([CallerMemberName] string method = "")

internal static string BuildQuotedString(string quotePrefix, string quoteSuffix, string unQuotedString)
{
var resultString = new StringBuilder();
var resultString = new StringBuilder(unQuotedString.Length + quoteSuffix.Length + quoteSuffix.Length);
AppendQuotedString(resultString, quotePrefix, quoteSuffix, unQuotedString);
return resultString.ToString();
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
}

internal static string AppendQuotedString(StringBuilder buffer, string quotePrefix, string quoteSuffix, string unQuotedString)
{

if (!string.IsNullOrEmpty(quotePrefix))
{
resultString.Append(quotePrefix);
buffer.Append(quotePrefix);
}

// Assuming that the suffix is escaped by doubling it. i.e. foo"bar becomes "foo""bar".
if (!string.IsNullOrEmpty(quoteSuffix))
{
resultString.Append(unQuotedString.Replace(quoteSuffix, quoteSuffix + quoteSuffix));
resultString.Append(quoteSuffix);
int start = buffer.Length;
buffer.Append(unQuotedString);
buffer.Replace(quoteSuffix, quoteSuffix + quoteSuffix, start, unQuotedString.Length);
buffer.Append(quoteSuffix);
}
else
{
resultString.Append(unQuotedString);
buffer.Append(unQuotedString);
}

return resultString.ToString();
return buffer.ToString();
}

static internal string BuildMultiPartName(string[] strings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3084,6 +3084,12 @@ internal void DeriveParameters()
p.TypeName = r[colNames[(int)ProcParamsColIndex.TypeCatalogName]] + "." +
r[colNames[(int)ProcParamsColIndex.TypeSchemaName]] + "." +
r[colNames[(int)ProcParamsColIndex.TypeName]];

// the constructed type name above is incorrectly formatted, it should be a 2 part name not 3
// for compatibility we can't change this because the bug has existed for a long time and been
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
// worked around by users, so identify that it is present and catch it later in the execution
// process once users can no longer interact with with the parameter type name
p.IsDerivedParameterTypeName = true;
}

// XmlSchema name for Xml types
Expand Down Expand Up @@ -5534,6 +5540,23 @@ private void SetUpRPCParameters(_SqlRPC rpc, bool inSchema, SqlParameterCollecti
{
options |= TdsEnums.RPC_PARAM_DEFAULT;
}

// detect incorrectly derived type names unchanged by the caller and fix them
if (parameter.IsDerivedParameterTypeName)
{
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(parameter.TypeName, "[\"", "]\"", Strings.SQL_TDSParserTableName, false);
if (parts != null && parts.Length == 4) // will always return int[4] right justified
{
if (
parts[3] != null && // name must not be null
parts[2] != null && // schema must not be null
parts[1] != null // server should not be null or we don't need to remove it
)
{
parameter.TypeName = QuoteIdentifier(parts.AsSpan(2, 2));
}
}
}
}

rpc.userParamMap[userParamCount] = ((((long)options) << 32) | (long)index);
Expand Down Expand Up @@ -5974,7 +5997,30 @@ internal string BuildParamList(TdsParser parser, SqlParameterCollection paramete
private static string ParseAndQuoteIdentifier(string identifier, bool isUdtTypeName)
{
string[] strings = SqlParameter.ParseTypeName(identifier, isUdtTypeName);
return ADP.BuildMultiPartName(strings);
return QuoteIdentifier(strings);
}

private static string QuoteIdentifier(ReadOnlySpan<string> strings)
{
StringBuilder bld = new StringBuilder();

// Stitching back together is a little tricky. Assume we want to build a full multi-part name
// with all parts except trimming separators for leading empty names (null or empty strings,
// but not whitespace). Separators in the middle should be added, even if the name part is
// null/empty, to maintain proper location of the parts.
for (int i = 0; i < strings.Length; i++)
{
if (0 < bld.Length)
{
bld.Append('.');
}
if (null != strings[i] && 0 != strings[i].Length)
{
ADP.AppendQuotedString(bld, "[", "]", strings[i]);
}
}

return bld.ToString();
}

// returns set option text to turn on format only and key info on and off
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,11 @@ public string UdtTypeName
public string TypeName
{
get => _typeName ?? ADP.StrEmpty;
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
set => _typeName = value;
set
{
_typeName = value;
IsDerivedParameterTypeName = false;
}
}

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlParameter.xml' path='docs/members[@name="SqlParameter"]/Value/*' />
Expand Down Expand Up @@ -964,6 +968,8 @@ internal string ParameterNameFixed

internal INullable ValueAsINullable => _valueAsINullable;

internal bool IsDerivedParameterTypeName { get; set; }

private void CloneHelper(SqlParameter destination)
{
// NOTE: _parent is not cloned
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2334,26 +2334,34 @@ static internal string MachineName()
return Environment.MachineName;
}

static internal string BuildQuotedString(string quotePrefix, string quoteSuffix, string unQuotedString)
internal static string BuildQuotedString(string quotePrefix, string quoteSuffix, string unQuotedString)
{
StringBuilder resultString = new StringBuilder();
if (ADP.IsEmpty(quotePrefix) == false)
var resultString = new StringBuilder(unQuotedString.Length + quoteSuffix.Length + quoteSuffix.Length);
AppendQuotedString(resultString, quotePrefix, quoteSuffix, unQuotedString);
return resultString.ToString();
}

internal static string AppendQuotedString(StringBuilder buffer, string quotePrefix, string quoteSuffix, string unQuotedString)
{
if (!string.IsNullOrEmpty(quotePrefix))
{
resultString.Append(quotePrefix);
buffer.Append(quotePrefix);
}

// Assuming that the suffix is escaped by doubling it. i.e. foo"bar becomes "foo""bar".
if (ADP.IsEmpty(quoteSuffix) == false)
if (!string.IsNullOrEmpty(quoteSuffix))
{
resultString.Append(unQuotedString.Replace(quoteSuffix, quoteSuffix + quoteSuffix));
resultString.Append(quoteSuffix);
int start = buffer.Length;
buffer.Append(unQuotedString);
buffer.Replace(quoteSuffix, quoteSuffix + quoteSuffix, start, unQuotedString.Length);
buffer.Append(quoteSuffix);
}
else
{
resultString.Append(unQuotedString);
buffer.Append(unQuotedString);
}

return resultString.ToString();
return buffer.ToString();
}

static internal string BuildMultiPartName(string[] strings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using System.Buffers;
using Microsoft.Data.Common;
using Microsoft.Data.Sql;
using Microsoft.Data.SqlClient.Server;
Expand Down Expand Up @@ -3538,6 +3539,12 @@ internal void DeriveParameters()
p.TypeName = r[colNames[(int)ProcParamsColIndex.TypeCatalogName]] + "." +
r[colNames[(int)ProcParamsColIndex.TypeSchemaName]] + "." +
r[colNames[(int)ProcParamsColIndex.TypeName]];

// the constructed type name above is incorrectly formatted, it should be a 2 part name not 3
// for compatibility we can't change this because the bug has existed for a long time and been
// worked around by users, so identify that it is present and catch it later in the execution
// process once users can no longer interact with with the parameter type name
p.IsDerivedParameterTypeName = true;
}

// XmlSchema name for Xml types
Expand Down Expand Up @@ -6464,6 +6471,23 @@ private void SetUpRPCParameters(_SqlRPC rpc, int startCount, bool inSchema, SqlP
{
rpc.paramoptions[j] |= TdsEnums.RPC_PARAM_DEFAULT;
}

// detect incorrectly derived type names unchanged by the caller and fix them
if (parameter.IsDerivedParameterTypeName)
{
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(parameter.TypeName, "[\"", "]\"", Strings.SQL_TDSParserTableName, false);
if (parts != null && parts.Length == 4) // will always return int[4] right justified
{
if (
parts[3] != null && // name must not be null
parts[2] != null && // schema must not be null
parts[1] != null // server should not be null or we don't need to remove it
)
{
parameter.TypeName = QuoteIdentifier(parts, 2, 2);
}
}
}
}

// Must set parameter option bit for LOB_COOKIE if unfilled LazyMat blob
Expand Down Expand Up @@ -6937,6 +6961,29 @@ private string ParseAndQuoteIdentifier(string identifier, bool isUdtTypeName)
return ADP.BuildMultiPartName(strings);
}

private static string QuoteIdentifier(string[] strings, int offset, int length)
{
StringBuilder bld = new StringBuilder();

// Stitching back together is a little tricky. Assume we want to build a full multi-part name
// with all parts except trimming separators for leading empty names (null or empty strings,
// but not whitespace). Separators in the middle should be added, even if the name part is
// null/empty, to maintain proper location of the parts.
for (int i = offset; i < (offset + length); i++)
{
if (0 < bld.Length)
{
bld.Append('.');
}
if (null != strings[i] && 0 != strings[i].Length)
{
ADP.AppendQuotedString(bld, "[", "]", strings[i]);
}
}

return bld.ToString();
}

// returns set option text to turn on format only and key info on and off
// When we are executing as a text command, then we never need
// to turn off the options since they command text is executed in the scope of sp_executesql.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,11 @@ public string UdtTypeName
public string TypeName
{
get => _typeName ?? ADP.StrEmpty;
set => _typeName = value;
set
{
_typeName = value;
IsDerivedParameterTypeName = false;
}
}

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlParameter.xml' path='docs/members[@name="SqlParameter"]/Value/*' />
Expand Down Expand Up @@ -955,6 +959,8 @@ internal string ParameterNameFixed

internal INullable ValueAsINullable => _valueAsINullable;

internal bool IsDerivedParameterTypeName { get; set; }

private void CloneHelper(SqlParameter destination)
{
// NOTE: _parent is not cloned
Expand Down
107 changes: 107 additions & 0 deletions src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UdtTest2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,113 @@ Func<int, SqlUserDefinedAggregateAttribute> create
() => create(-2),
errorMessage);
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsUdtTestDatabasePresent), nameof(DataTestUtility.AreConnStringsSetup))]
public void UDTParams_DeriveParameters_CheckAutoFixSuccess()
{
// the type and sproc must be commited to the database or this test will deadlock with a schema lock violation
// if you are missing these database entities then you should look for an updated version of the database creation script

string sprocName = "sp_insert_customers";
string typeName = "CustomerAddress";
string customerAddressTypeIncorrectName = $"{DataTestUtility.UdtTestDbName}.dbo.{typeName.Trim('[', ']')}";
string customerAddressTypeCorrectedName = $"[dbo].[{typeName.Trim('[', ']')}]";
string customerParameterName = "@customers";

Address addr = Address.Parse("123 baker st || Redmond");
DataTable table = new DataTable();
table.Columns.Add();
table.Columns.Add();
table.Rows.Add("john", addr);

using (SqlConnection connection = new SqlConnection(_connStr))
{
connection.Open();
using (SqlTransaction transaction = connection.BeginTransaction())
using (SqlCommand cmd = new SqlCommand(sprocName, connection, transaction))
{
try
{
cmd.CommandType = CommandType.StoredProcedure;

SqlCommandBuilder.DeriveParameters(cmd);

Assert.NotNull(cmd.Parameters);
Assert.Equal(2, cmd.Parameters.Count); // [return_value, table]

SqlParameter p = cmd.Parameters[1];

Assert.Equal(customerParameterName, p.ParameterName);
Assert.Equal(SqlDbType.Structured, p.SqlDbType);
Assert.Equal(customerAddressTypeIncorrectName, p.TypeName); // the 3 part name is incorrect but needs to be maintained for compatibility
p.Value = table;

cmd.ExecuteNonQuery();

Assert.Equal(customerAddressTypeCorrectedName, p.TypeName); // check that the auto fix has been applied correctly
}
finally
{
transaction.Rollback();
}
}
}
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsUdtTestDatabasePresent), nameof(DataTestUtility.AreConnStringsSetup))]
public void UDTParams_DeriveParameters_CheckAutoFixOverride()
{
// the type and sproc must be commited to the database or this test will deadlock with a schema lock violation
// if you are missing these database entities then you should look for an updated version of the database creation script

string sprocName = "sp_insert_customers";
string typeName = "CustomerAddress";
string customerAddressTypeIncorrectName = $"{DataTestUtility.UdtTestDbName}.dbo.{typeName.Trim('[', ']')}";
string customerAddressTypeCorrectedName = $"[dbo].[{typeName.Trim('[', ']')}]";
string customerParameterName = "@customers";

Address addr = Address.Parse("123 baker st || Redmond");
DataTable table = new DataTable();
table.Columns.Add();
table.Columns.Add();
table.Rows.Add("john", addr);

using (SqlConnection connection = new SqlConnection(_connStr))
{
connection.Open();
using (SqlTransaction transaction = connection.BeginTransaction())
using (SqlCommand cmd = new SqlCommand(sprocName, connection, transaction))
{
try
{
cmd.CommandType = CommandType.StoredProcedure;

SqlCommandBuilder.DeriveParameters(cmd);

Assert.NotNull(cmd.Parameters);
Assert.Equal(2, cmd.Parameters.Count); // [return_value, table]

SqlParameter p = cmd.Parameters[1];

Assert.Equal(customerParameterName, p.ParameterName);
Assert.Equal(SqlDbType.Structured, p.SqlDbType);
Assert.Equal(customerAddressTypeIncorrectName, p.TypeName); // the 3 part name is incorrect but needs to be maintained for compatibility
p.Value = table;

p.TypeName = customerAddressTypeIncorrectName; // force using the incorrect name by manually setting it

SqlException exception = Assert.Throws<SqlException>(
() => cmd.ExecuteNonQuery()
);
Assert.Contains("Database name is not allowed", exception.Message);
}
finally
{
transaction.Rollback();
}
}
}
}
}
}

Binary file modified tools/testsql/createUdtTestDb.sql
Binary file not shown.