Merge | TdsParserStateObject lifetime-related methods (#3394)

* Port PacketHandle to netfx

* Remove PacketHandle alias shim

* Align netfx use of PacketHandle

* Merge IsPacketEmpty

* Merge ReleasePacket

* Merge ReadAsync, ReadSyncOverAsync

Also move ReadSyncOverAsync down in netcore's TdsParserStateObjectNative to aid later merge

* Merge IsFailedHandle

* Merge SniPacketGetData

* Merge EmptyReadPacket

Also reorder member in TdsParserStateObjectNative to simplify later merge

* Merge CheckPacket

* Merge WritePacket, IsValidPacket

* Merge GetResetWritePacket

* Merge ClearAllWritePackets, AddPacketToPendingList, RemovePacketFromPendingList

* Improve diff between versions of TdsParserStateObjectNative

* PR feedback from #3353 - format IsValidPacket

* Address merge conflicts

* Merge DisposePacketCache

This also allows _writePacketCache to be migrated to TdsParserStateObjectNative in netfx.

* Merge CreateSessionHandle

* Move AssignPendingDNSInfo from TdsParser, merge

* Sync method signature for CreatePhysicalSNIHandle

* Port netcore _serverSpn array to netfx

* Merge CreatePhysicalSNIHandle

* Refactor to insert FreeGcHandle

* Move CreateConsumerInfo and _gcHandle to TdsParserStateObjectNative

* Merge Dispose

* Add region to both TdsParserStateObjectNative files

* Make public constructor internal

* Remove redundant assignment to _serverSpn

* Remove uncalled BestEffortCleanup methods

This allows TdsParser.netfx.cs to be deleted

---------

Co-authored-by: Ben Russell <[email protected]>

Author: edwardneal

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

@@ -434,6 +434,8 @@ internal void Connect(ServerInfo serverInfo,
                 false,
                 true,
                 fParallel,
+                TransparentNetworkResolutionState.DisabledMode,
+                -1,
                 _connHandler.ConnectionOptions.IPAddressPreference,
                 FQDNforDNSCache,
                 ref _connHandler.pendingSQLDNSObject,
@@ -532,6 +534,8 @@ internal void Connect(ServerInfo serverInfo,
                     true,
                     true,
                     fParallel,
+                    TransparentNetworkResolutionState.DisabledMode,
+                    -1,
                     _connHandler.ConnectionOptions.IPAddressPreference,
                     FQDNforDNSCache,
                     ref _connHandler.pendingSQLDNSObject,

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

@@ -21,7 +21,7 @@ internal abstract partial class TdsParserStateObject
         // Constructors //
         //////////////////
 
-        internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async)
+        protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async)
         {
             // Construct a MARS session
             Debug.Assert(parser != null, "no parser?");
@@ -57,40 +57,17 @@ internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCon
         // General methods //
         /////////////////////
 
-        internal abstract void CreatePhysicalSNIHandle(
-            string serverName,
-            TimeoutTimer timeout,
-            out byte[] instanceName,
-            out ResolvedServerSpn resolvedSpn,
-            bool flushCache,
-            bool async,
-            bool fParallel,
-            SqlConnectionIPAddressPreference iPAddressPreference,
-            string cachedFQDN,
-            ref SQLDNSInfo pendingDNSInfo,
-            string serverSPN,
-            bool isIntegratedSecurity = false,
-            bool tlsFirst = false,
-            string hostNameInCertificate = "",
-            string serverCertificateFilename = "");
-
-        internal abstract void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo);
-
-        protected abstract void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async);
-
-        protected abstract void FreeGcHandle(int remaining, bool release);
-
         internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename);
 
-        internal abstract void Dispose();
-
         internal abstract uint CheckConnection();
 
         internal int DecrementPendingCallbacks(bool release)
         {
             int remaining = Interlocked.Decrement(ref _pendingCallbacks);
             SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.DecrementPendingCallbacks | ADV | State Object Id {0}, after decrementing _pendingCallbacks: {1}", _objectID, _pendingCallbacks);
+            
             FreeGcHandle(remaining, release);
+
             // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed
             // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it
             Debug.Assert((remaining == -1 && SessionHandle.IsNull) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}");

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

@@ -85,6 +85,8 @@ internal override void CreatePhysicalSNIHandle(
             bool flushCache,
             bool async,
             bool parallel,
+            TransparentNetworkResolutionState transparentNetworkResolutionState,
+            int totalTimeout,
             SqlConnectionIPAddressPreference iPAddressPreference,
             string cachedFQDN,
             ref SQLDNSInfo pendingDNSInfo,

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

@@ -52,14 +52,12 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi
         {
         }
 
-        public TdsParserStateObjectNative(TdsParser parser)
+        internal TdsParserStateObjectNative(TdsParser parser)
             : base(parser)
         {
         }
 
-        ////////////////
-        // Properties //
-        ////////////////
+        #region Properties
 
         internal SNIHandle Handle => _sessionHandle;
 
@@ -71,6 +69,8 @@ public TdsParserStateObjectNative(TdsParser parser)
 
         internal override Guid? SessionId => default;
 
+        #endregion
+
         protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async)
         {
             Debug.Assert(physicalConnection is TdsParserStateObjectNative, "Expected a stateObject of type " + this.GetType());
@@ -83,6 +83,9 @@ protected override void CreateSessionHandle(TdsParserStateObject physicalConnect
             _sessionHandle = new SNIHandle(myInfo, nativeSNIObject.Handle, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo);
         }
 
+        // Retrieve the IP and port number from native SNI for TCP protocol. The IP information is stored temporarily in the
+        // pendingSQLDNSObject but not in the DNS Cache at this point. We only add items to the DNS Cache after we receive the
+        // IsSupported flag as true in the feature ext ack from server.
         internal override void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo)
         {
             uint result;
@@ -155,11 +158,13 @@ internal override void CreatePhysicalSNIHandle(
             string serverName,
             TimeoutTimer timeout,
             out byte[] instanceName,
-            out Microsoft.Data.SqlClient.ManagedSni.ResolvedServerSpn resolvedSpn,
+            out ManagedSni.ResolvedServerSpn resolvedSpn,
             bool flushCache,
             bool async,
             bool fParallel,
-            SqlConnectionIPAddressPreference ipPreference,
+            TransparentNetworkResolutionState transparentNetworkResolutionState,
+            int totalTimeout,
+            SqlConnectionIPAddressPreference iPAddressPreference,
             string cachedFQDN,
             ref SQLDNSInfo pendingDNSInfo,
             string serverSPN,
@@ -188,7 +193,7 @@ internal override void CreatePhysicalSNIHandle(
             bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out cachedDNSInfo);
 
             _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName,
-                flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate);
+                flushCache, !async, fParallel, iPAddressPreference, cachedDNSInfo, hostNameInCertificate);
             resolvedSpn = new(serverSPN.TrimEnd());
         }
 

src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj

@@ -450,6 +450,9 @@
     <Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\LocalAppContextSwitches.cs">
       <Link>Microsoft\Data\SqlClient\LocalAppContextSwitches.cs</Link>
     </Compile>
+    <Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ManagedSni\ResolvedServerSpn.cs">
+      <Link>Microsoft\Data\SqlClient\ManagedSni\ResolvedServerSpn.cs</Link>
+    </Compile>
     <Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\NoneAttestationEnclaveProvider.cs">
       <Link>Microsoft\Data\SqlClient\NoneAttestationEnclaveProvider.cs</Link>
     </Compile>
@@ -924,7 +927,6 @@
     <Compile Include="Microsoft\Data\SqlClient\SqlConnectionHelper.cs" />
     <Compile Include="Microsoft\Data\SqlClient\SqlInternalConnectionTds.cs" />
     <Compile Include="Microsoft\Data\SqlClient\TdsParser.cs" />
-    <Compile Include="Microsoft\Data\SqlClient\TdsParser.netfx.cs" />
     <Compile Include="Microsoft\Data\SqlClient\TdsParserStateObject.netfx.cs" />
     <Compile Include="Microsoft\Data\SqlClient\TdsParserStateObjectNative.cs" />
   </ItemGroup>

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

@@ -381,8 +381,11 @@ internal void Connect(ServerInfo serverInfo,
                 ThrowExceptionAndWarning(_physicalStateObj);
                 Debug.Fail("SNI returned status != success, but no error thrown?");
             }
-
-            string serverSpn = null;
+            else
+            {
+                SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID,
+                    authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString());
+            }
 
             //Create LocalDB instance if necessary
             if (connHandler.ConnectionOptions.LocalDBInstance != null)
@@ -401,17 +404,6 @@ internal void Connect(ServerInfo serverInfo,
             {
                 _authenticationProvider = Connection._sspiContextProvider ?? _physicalStateObj.CreateSspiContextProvider();
 
-                if (!string.IsNullOrEmpty(serverInfo.ServerSPN))
-                {
-                    serverSpn = serverInfo.ServerSPN;
-                    SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Server SPN `{0}` from the connection string is used.", serverInfo.ServerSPN);
-                }
-                else
-                {
-                    // Empty signifies to interop layer that SPN needs to be generated
-                    serverSpn = string.Empty;
-                }
-
                 SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> SSPI or Active Directory Authentication Library for SQL Server based integrated authentication");
             }
             else
@@ -495,15 +487,20 @@ internal void Connect(ServerInfo serverInfo,
                 serverInfo.ExtendedServerName,
                 timeout,
                 out instanceName,
-                ref serverSpn,
+                out var resolvedServerSpn,
                 false,
                 true,
                 fParallel,
                 transparentNetworkResolutionState,
                 totalTimeout,
                 _connHandler.ConnectionOptions.IPAddressPreference,
                 FQDNforDNSCache,
-                hostNameInCertificate);
+                ref _connHandler.pendingSQLDNSObject,
+                serverInfo.ServerSPN,
+                integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated,
+                isTlsFirst,
+                hostNameInCertificate,
+                serverCertificateFilename);
 
             if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status)
             {
@@ -544,7 +541,7 @@ internal void Connect(ServerInfo serverInfo,
             Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionId");
 
             // for DNS Caching phase 1
-            AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache);
+            _physicalStateObj.AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject);
 
             if (!ClientOSEncryptionSupport)
             {
@@ -587,15 +584,20 @@ internal void Connect(ServerInfo serverInfo,
                     serverInfo.ExtendedServerName,
                     timeout,
                     out instanceName,
-                    ref serverSpn,
+                    out resolvedServerSpn,
                     true,
                     true,
                     fParallel,
                     transparentNetworkResolutionState,
                     totalTimeout,
                     _connHandler.ConnectionOptions.IPAddressPreference,
-                    serverInfo.ResolvedServerName,
-                    hostNameInCertificate);
+                    FQDNforDNSCache,
+                    ref _connHandler.pendingSQLDNSObject,
+                    serverInfo.ServerSPN,
+                    integratedSecurity,
+                    isTlsFirst,
+                    hostNameInCertificate,
+                    serverCertificateFilename);
 
                 if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status)
                 {
@@ -609,7 +611,7 @@ internal void Connect(ServerInfo serverInfo,
                 SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Sending prelogin handshake");
 
                 // for DNS Caching phase 1
-                AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache);
+                _physicalStateObj.AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject);
 
                 SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCertificateFilename);
                 status = ConsumePreLoginHandshake(
@@ -631,7 +633,10 @@ internal void Connect(ServerInfo serverInfo,
             }
             SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Prelogin handshake successful");
 
-            _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpn);
+            if (_authenticationProvider is { })
+            {
+                _authenticationProvider.Initialize(serverInfo, _physicalStateObj, this, resolvedServerSpn.Primary, resolvedServerSpn.Secondary);
+            }
 
             if (_fMARS && marsCapable)
             {