Fix derived parameters containing typename incorrectly (#1020)

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/Common/AdapterUtil.cs

@@ -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();
+        }
+
+        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)

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs

@@ -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 
+                        // 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
@@ -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);
@@ -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

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlParameter.cs

@@ -669,7 +669,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/*' />
@@ -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

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/AdapterUtil.cs

@@ -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)

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs

@@ -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;
@@ -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
@@ -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
@@ -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.

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlParameter.cs

@@ -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/*' />
@@ -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

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UdtTest2.cs

@@ -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();
+                    }
+                }
+            }
+        }
     }
 }