Address TSA Scan issues (#586)

cheenamalhotra · web-flow · commit 245db93e8cc1 · 2020-06-01T15:45:52.000-07:00
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionPool.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionPool.cs
@@ -1557,7 +1557,7 @@ internal void PutObject(DbConnectionInternal obj, object owningObject)
                 // once we leave the lock, because it sets _pooledCount such
                 // that it won't appear to be out of the pool.  What that
                 // means, is that we're now responsible for this connection:
-                // it won't get reclaimed if we drop the ball somewhere.
+                // it won't get reclaimed if it gets lost.
                 obj.PrePush(owningObject);
             }
 
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionString.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
@@ -332,8 +332,8 @@ internal SqlConnectionString(string connectionString) : base(connectionString, G
             ValidateValueLength(_dataSource, TdsEnums.MAXLEN_SERVERNAME, KEY.Data_Source);
             ValidateValueLength(_failoverPartner, TdsEnums.MAXLEN_SERVERNAME, KEY.FailoverPartner);
             ValidateValueLength(_initialCatalog, TdsEnums.MAXLEN_DATABASE, KEY.Initial_Catalog);
-            ValidateValueLength(_password, TdsEnums.MAXLEN_PASSWORD, KEY.Password);
-            ValidateValueLength(_userID, TdsEnums.MAXLEN_USERNAME, KEY.User_ID);
+            ValidateValueLength(_password, TdsEnums.MAXLEN_CLIENTSECRET, KEY.Password);
+            ValidateValueLength(_userID, TdsEnums.MAXLEN_CLIENTID, KEY.User_ID);
             if (null != _workstationId)
             {
                 ValidateValueLength(_workstationId, TdsEnums.MAXLEN_HOSTNAME, KEY.Workstation_Id);
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCredential.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCredential.cs
@@ -21,19 +21,19 @@ public SqlCredential(string userId, SecureString password)
                 throw ADP.ArgumentNull(nameof(userId));
             }
 
-            if (userId.Length > TdsEnums.MAXLEN_USERNAME)
+            if (userId.Length > TdsEnums.MAXLEN_CLIENTID)
             {
-                throw ADP.InvalidArgumentLength(nameof(userId), TdsEnums.MAXLEN_USERNAME);
+                throw ADP.InvalidArgumentLength(nameof(userId), TdsEnums.MAXLEN_CLIENTID);
             }
 
             if (password == null)
             {
                 throw ADP.ArgumentNull(nameof(password));
             }
 
-            if (password.Length > TdsEnums.MAXLEN_PASSWORD)
+            if (password.Length > TdsEnums.MAXLEN_CLIENTSECRET)
             {
-                throw ADP.InvalidArgumentLength(nameof(password), TdsEnums.MAXLEN_PASSWORD);
+                throw ADP.InvalidArgumentLength(nameof(password), TdsEnums.MAXLEN_CLIENTSECRET);
             }
 
             if (!password.IsReadOnly())
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsEnums.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsEnums.cs
@@ -603,8 +603,8 @@ public enum ActiveDirectoryWorkflow : byte
         // Login data validation Rules
         //
         internal const ushort MAXLEN_HOSTNAME = 128; // the client machine name
-        internal const ushort MAXLEN_USERNAME = 128; // the client user id
-        internal const ushort MAXLEN_PASSWORD = 128; // the password supplied by the client
+        internal const ushort MAXLEN_CLIENTID = 128;
+        internal const ushort MAXLEN_CLIENTSECRET = 128;
         internal const ushort MAXLEN_APPNAME = 128; // the client application name
         internal const ushort MAXLEN_SERVERNAME = 128; // the server name
         internal const ushort MAXLEN_CLIENTINTERFACE = 128; // the interface library name
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
@@ -2444,7 +2444,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead
             // we throw an Operation Cancelled error
             if (stateObj.HasReceivedAttention)
             {
-                // Dev11 #344723: SqlClient stress hang System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
+                // Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
                 // Spin until SendAttention has cleared _attentionSending, this prevents a race condition between receiving the attention ACK and setting _attentionSent
                 TryRunSetupSpinWaitContinuation(stateObj);
 
@@ -7798,11 +7798,11 @@ internal void TdsLogin(SqlLogin rec, TdsEnums.FeatureExtension requestedFeatures
             Debug.Assert(fedAuthFeatureExtensionData == null || (requestedFeatures & TdsEnums.FeatureExtension.FedAuth) != 0, "fedAuthFeatureExtensionData provided without fed auth feature request");
             Debug.Assert(fedAuthFeatureExtensionData != null || (requestedFeatures & TdsEnums.FeatureExtension.FedAuth) == 0, "Fed Auth feature requested without specifying fedAuthFeatureExtensionData.");
 
-            Debug.Assert(rec.userName == null || (rec.userName != null && TdsEnums.MAXLEN_USERNAME >= rec.userName.Length), "_userID.Length exceeds the max length for this value");
-            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_USERNAME >= rec.credential.UserId.Length), "_credential.UserId.Length exceeds the max length for this value");
+            Debug.Assert(rec.userName == null || (rec.userName != null && TdsEnums.MAXLEN_CLIENTID >= rec.userName.Length), "_userID.Length exceeds the max length for this value");
+            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_CLIENTID >= rec.credential.UserId.Length), "_credential.UserId.Length exceeds the max length for this value");
 
-            Debug.Assert(rec.password == null || (rec.password != null && TdsEnums.MAXLEN_PASSWORD >= rec.password.Length), "_password.Length exceeds the max length for this value");
-            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_PASSWORD >= rec.credential.Password.Length), "_credential.Password.Length exceeds the max length for this value");
+            Debug.Assert(rec.password == null || (rec.password != null && TdsEnums.MAXLEN_CLIENTSECRET >= rec.password.Length), "_password.Length exceeds the max length for this value");
+            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_CLIENTSECRET >= rec.credential.Password.Length), "_credential.Password.Length exceeds the max length for this value");
 
             Debug.Assert(rec.credential != null || rec.userName != null || rec.password != null, "cannot mix the new secure password system and the connection string based password");
             Debug.Assert(rec.newSecurePassword != null || rec.newPassword != null, "cannot have both new secure change password and string based change password");
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
@@ -3465,7 +3465,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)
 
                 try
                 {
-                    // Dev11 #344723: SqlClient stress hang System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
+                    // Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
                     // Set _attentionSending to true before sending attention and reset after setting _attentionSent
                     // This prevents a race condition between receiving the attention ACK and setting _attentionSent
                     _attentionSending = true;
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionPool.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionPool.cs
@@ -1820,7 +1820,7 @@ internal void PutObject(DbConnectionInternal obj, object owningObject)
                 // once we leave the lock, because it sets _pooledCount such
                 // that it won't appear to be out of the pool.  What that
                 // means, is that we're now responsible for this connection:
-                // it won't get reclaimed if we drop the ball somewhere.
+                // it won't get reclaimed if it gets lost.
                 obj.PrePush(owningObject);
 
                 // TODO: Consider using a Cer to ensure that we mark the object for reclaimation in the event something bad happens?
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionString.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
@@ -415,8 +415,8 @@ internal SqlConnectionString(string connectionString) : base(connectionString, G
             ValidateValueLength(_dataSource, TdsEnums.MAXLEN_SERVERNAME, KEY.Data_Source);
             ValidateValueLength(_failoverPartner, TdsEnums.MAXLEN_SERVERNAME, KEY.FailoverPartner);
             ValidateValueLength(_initialCatalog, TdsEnums.MAXLEN_DATABASE, KEY.Initial_Catalog);
-            ValidateValueLength(_password, TdsEnums.MAXLEN_PASSWORD, KEY.Password);
-            ValidateValueLength(_userID, TdsEnums.MAXLEN_USERNAME, KEY.User_ID);
+            ValidateValueLength(_password, TdsEnums.MAXLEN_CLIENTSECRET, KEY.Password);
+            ValidateValueLength(_userID, TdsEnums.MAXLEN_CLIENTID, KEY.User_ID);
             if (null != _workstationId)
             {
                 ValidateValueLength(_workstationId, TdsEnums.MAXLEN_HOSTNAME, KEY.Workstation_Id);
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCredential.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCredential.cs
@@ -28,19 +28,19 @@ public SqlCredential(string userId, SecureString password)
                 throw ADP.ArgumentNull("userId");
             }
 
-            if (userId.Length > TdsEnums.MAXLEN_USERNAME)
+            if (userId.Length > TdsEnums.MAXLEN_CLIENTID)
             {
-                throw ADP.InvalidArgumentLength("userId", TdsEnums.MAXLEN_USERNAME);
+                throw ADP.InvalidArgumentLength("userId", TdsEnums.MAXLEN_CLIENTID);
             }
 
             if (password == null)
             {
                 throw ADP.ArgumentNull("password");
             }
 
-            if (password.Length > TdsEnums.MAXLEN_PASSWORD)
+            if (password.Length > TdsEnums.MAXLEN_CLIENTSECRET)
             {
-                throw ADP.InvalidArgumentLength("password", TdsEnums.MAXLEN_PASSWORD);
+                throw ADP.InvalidArgumentLength("password", TdsEnums.MAXLEN_CLIENTSECRET);
             }
 
             if (!password.IsReadOnly())
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs
@@ -3412,7 +3412,7 @@ private bool TryHasMoreResults(out bool moreResults)
                             return true;
                     }
 
-                    // Dev11 Bug 316483:Hang on SqlDataReader::TryHasMoreResults using MARS
+                    // Dev11 Bug 316483: Stuck at SqlDataReader::TryHasMoreResults using MARS
                     // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=316483
                     // TryRun() will immediately return if the TdsParser is closed\broken, causing us to enter an infinite loop
                     // Instead, we will throw a closed connection exception
@@ -3497,7 +3497,7 @@ private bool TryHasMoreRows(out bool moreRows)
                             ParsedDoneToken = true;
                         }
 
-                        // Dev11 Bug 316483:Hang on SqlDataReader::TryHasMoreResults using MARS
+                        // Dev11 Bug 316483: Stuck at SqlDataReader::TryHasMoreResults when using MARS
                         // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=316483
                         // TryRun() will immediately return if the TdsParser is closed\broken, causing us to enter an infinite loop
                         // Instead, we will throw a closed connection exception
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsEnums.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsEnums.cs
@@ -603,8 +603,8 @@ public enum ActiveDirectoryWorkflow : byte
         // Login data validation Rules
         //
         internal const ushort MAXLEN_HOSTNAME = 128; // the client machine name
-        internal const ushort MAXLEN_USERNAME = 128; // the client user id
-        internal const ushort MAXLEN_PASSWORD = 128; // the password supplied by the client
+        internal const ushort MAXLEN_CLIENTID = 128; 
+        internal const ushort MAXLEN_CLIENTSECRET = 128; 
         internal const ushort MAXLEN_APPNAME = 128; // the client application name
         internal const ushort MAXLEN_SERVERNAME = 128; // the server name
         internal const ushort MAXLEN_CLIENTINTERFACE = 128; // the interface library name
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
@@ -2778,7 +2778,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead
             // we throw an Operation Cancelled error
             if (stateObj._attentionReceived)
             {
-                // Dev11 #344723: SqlClient stress hang System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
+                // Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
                 // Spin until SendAttention has cleared _attentionSending, this prevents a race condition between receiving the attention ACK and setting _attentionSent
                 SpinWait.SpinUntil(() => !stateObj._attentionSending);
 
@@ -8546,11 +8546,11 @@ internal void TdsLogin(SqlLogin rec,
             Debug.Assert(fedAuthFeatureExtensionData == null || (requestedFeatures & TdsEnums.FeatureExtension.FedAuth) != 0, "fedAuthFeatureExtensionData provided without fed auth feature request");
             Debug.Assert(fedAuthFeatureExtensionData != null || (requestedFeatures & TdsEnums.FeatureExtension.FedAuth) == 0, "Fed Auth feature requested without specifying fedAuthFeatureExtensionData.");
 
-            Debug.Assert(rec.userName == null || (rec.userName != null && TdsEnums.MAXLEN_USERNAME >= rec.userName.Length), "_userID.Length exceeds the max length for this value");
-            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_USERNAME >= rec.credential.UserId.Length), "_credential.UserId.Length exceeds the max length for this value");
+            Debug.Assert(rec.userName == null || (rec.userName != null && TdsEnums.MAXLEN_CLIENTID >= rec.userName.Length), "_userID.Length exceeds the max length for this value");
+            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_CLIENTID >= rec.credential.UserId.Length), "_credential.UserId.Length exceeds the max length for this value");
 
-            Debug.Assert(rec.password == null || (rec.password != null && TdsEnums.MAXLEN_PASSWORD >= rec.password.Length), "_password.Length exceeds the max length for this value");
-            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_PASSWORD >= rec.credential.Password.Length), "_credential.Password.Length exceeds the max length for this value");
+            Debug.Assert(rec.password == null || (rec.password != null && TdsEnums.MAXLEN_CLIENTSECRET >= rec.password.Length), "_password.Length exceeds the max length for this value");
+            Debug.Assert(rec.credential == null || (rec.credential != null && TdsEnums.MAXLEN_CLIENTSECRET >= rec.credential.Password.Length), "_credential.Password.Length exceeds the max length for this value");
 
             Debug.Assert(rec.credential != null || rec.userName != null || rec.password != null, "cannot mix the new secure password system and the connection string based password");
             Debug.Assert(rec.newSecurePassword != null || rec.newPassword != null, "cannot have both new secure change password and string based change password");
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
@@ -3490,7 +3490,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)
                 RuntimeHelpers.PrepareConstrainedRegions();
                 try
                 {
-                    // Dev11 #344723: SqlClient stress hang System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
+                    // Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
                     // Set _attentionSending to true before sending attention and reset after setting _attentionSent
                     // This prevents a race condition between receiving the attention ACK and setting _attentionSent
                     _attentionSending = true;
diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/Setup/CryptoVector.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/Setup/CryptoVector.cs
@@ -41,7 +41,7 @@ internal enum CryptNativeTestVectorType
         /// </summary>
         RsaKeyPair,
         /// <summary>
-        /// Special case of RSA test vector, which holds a PFX, password = @"P@zzw0rD!SqlvN3x+"
+        /// Special case of RSA test vector, which holds a PFX
         /// </summary>
         RsaPfx,
     }
diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs
@@ -24,7 +24,7 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
     [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
     public class DiagnosticTest
     {
-        private const string BadConnectionString = "data source = bad; initial catalog = bad; uid = bad; password = bad; connection timeout = 1;";
+        private const string BadConnectionString = "data source = bad; initial catalog = bad; integrated security = true; connection timeout = 1;";
         private static readonly string s_tcpConnStr = DataTestUtility.TCPConnectionString ?? string.Empty;
 
         [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]

Original file line number	Diff line number	Diff line change
`@@ -1557,7 +1557,7 @@ internal void PutObject(DbConnectionInternal obj, object owningObject)`
`1557`	`1557`	`// once we leave the lock, because it sets _pooledCount such`
`1558`	`1558`	`// that it won't appear to be out of the pool. What that`
`1559`	`1559`	`// means, is that we're now responsible for this connection:`
`1560`		`- // it won't get reclaimed if we drop the ball somewhere.`
	`1560`	`+ // it won't get reclaimed if it gets lost.`
`1561`	`1561`	`obj.PrePush(owningObject);`
`1562`	`1562`	`}`
`1563`	`1563`
Original file line number	Diff line number	Diff line change
`@@ -21,19 +21,19 @@ public SqlCredential(string userId, SecureString password)`
`21`	`21`	`throw ADP.ArgumentNull(nameof(userId));`
`22`	`22`	`}`
`23`	`23`
`24`		`- if (userId.Length > TdsEnums.MAXLEN_USERNAME)`
	`24`	`+ if (userId.Length > TdsEnums.MAXLEN_CLIENTID)`
`25`	`25`	`{`
`26`		`- throw ADP.InvalidArgumentLength(nameof(userId), TdsEnums.MAXLEN_USERNAME);`
	`26`	`+ throw ADP.InvalidArgumentLength(nameof(userId), TdsEnums.MAXLEN_CLIENTID);`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`if (password == null)`
`30`	`30`	`{`
`31`	`31`	`throw ADP.ArgumentNull(nameof(password));`
`32`	`32`	`}`
`33`	`33`
`34`		`- if (password.Length > TdsEnums.MAXLEN_PASSWORD)`
	`34`	`+ if (password.Length > TdsEnums.MAXLEN_CLIENTSECRET)`
`35`	`35`	`{`
`36`		`- throw ADP.InvalidArgumentLength(nameof(password), TdsEnums.MAXLEN_PASSWORD);`
	`36`	`+ throw ADP.InvalidArgumentLength(nameof(password), TdsEnums.MAXLEN_CLIENTSECRET);`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`if (!password.IsReadOnly())`
Original file line number	Diff line number	Diff line change
`@@ -3465,7 +3465,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)`
`3465`	`3465`
`3466`	`3466`	`try`
`3467`	`3467`	`{`
`3468`		`- // Dev11 #344723: SqlClient stress hang System_Data!Tcp::ReadSync via a call to SqlDataReader::Close`
	`3468`	`+ // Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close`
`3469`	`3469`	`// Set _attentionSending to true before sending attention and reset after setting _attentionSent`
`3470`	`3470`	`// This prevents a race condition between receiving the attention ACK and setting _attentionSent`
`3471`	`3471`	`_attentionSending = true;`
Original file line number	Diff line number	Diff line change
`@@ -28,19 +28,19 @@ public SqlCredential(string userId, SecureString password)`
`28`	`28`	`throw ADP.ArgumentNull("userId");`
`29`	`29`	`}`
`30`	`30`
`31`		`- if (userId.Length > TdsEnums.MAXLEN_USERNAME)`
	`31`	`+ if (userId.Length > TdsEnums.MAXLEN_CLIENTID)`
`32`	`32`	`{`
`33`		`- throw ADP.InvalidArgumentLength("userId", TdsEnums.MAXLEN_USERNAME);`
	`33`	`+ throw ADP.InvalidArgumentLength("userId", TdsEnums.MAXLEN_CLIENTID);`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`if (password == null)`
`37`	`37`	`{`
`38`	`38`	`throw ADP.ArgumentNull("password");`
`39`	`39`	`}`
`40`	`40`
`41`		`- if (password.Length > TdsEnums.MAXLEN_PASSWORD)`
	`41`	`+ if (password.Length > TdsEnums.MAXLEN_CLIENTSECRET)`
`42`	`42`	`{`
`43`		`- throw ADP.InvalidArgumentLength("password", TdsEnums.MAXLEN_PASSWORD);`
	`43`	`+ throw ADP.InvalidArgumentLength("password", TdsEnums.MAXLEN_CLIENTSECRET);`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`if (!password.IsReadOnly())`