Invalidate SmtpClient cached connection when connection-affecting properties change

src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpClient.cs

@@ -43,6 +43,7 @@ public class SmtpClient : IDisposable
         private bool _inCall;
         private bool _timedOut;
         private string? _targetName;
+        private const string DefaultTargetNamePrefix = "SMTPSVC/";
         private SmtpDeliveryMethod _deliveryMethod = SmtpDeliveryMethod.Network;
         private SmtpDeliveryFormat _deliveryFormat = SmtpDeliveryFormat.SevenBit; // Non-EAI default
         private string? _pickupDirectoryLocation;
@@ -103,7 +104,7 @@ private void Initialize()
                 _port = DefaultPort;
             }
 
-            _targetName ??= "SMTPSVC/" + _host;
+            _targetName ??= DefaultTargetNamePrefix + _host;
 
             if (_clientDomain == null)
             {
@@ -162,8 +163,20 @@ public string? Host
 
                 if (value != _host)
                 {
+                    // If TargetName is still the default derived from the current host, keep it in
+                    // sync with the new host so Negotiate/NTLM authentication uses the correct SPN.
+                    // A TargetName explicitly set by the caller (not matching the default) is left
+                    // untouched.
+                    if (_targetName == DefaultTargetNamePrefix + _host)
+                    {
+                        _targetName = DefaultTargetNamePrefix + value;
+                    }
+
                     _host = value;
                     _servicePoint = null;
+                    // The cached connection targets the previous host, so invalidate it to force
+                    // a new connection to be established on the next send.
+                    _transport.InvalidateCachedConnection();
                 }
             }
         }
@@ -187,6 +200,9 @@ public int Port
                 {
                     _port = value;
                     _servicePoint = null;
+                    // The cached connection targets the previous port, so invalidate it to force
+                    // a new connection to be established on the next send.
+                    _transport.InvalidateCachedConnection();
                 }
             }
         }
@@ -316,6 +332,11 @@ public bool EnableSsl
             }
             set
             {
+                if (_inCall)
+                {
+                    throw new InvalidOperationException(SR.SmtpInvalidOperationDuringSend);
+                }
+
                 _transport.EnableSsl = value;
             }
         }
@@ -334,7 +355,21 @@ public X509CertificateCollection ClientCertificates
         public string? TargetName
         {
             get { return _targetName; }
-            set { _targetName = value; }
+            set
+            {
+                if (_inCall)
+                {
+                    throw new InvalidOperationException(SR.SmtpInvalidOperationDuringSend);
+                }
+
+                if (value != _targetName)
+                {
+                    _targetName = value;
+                    // The target name is the SPN used during authentication, so invalidate any
+                    // cached connection to force a new one on the next send.
+                    _transport.InvalidateCachedConnection();
+                }
+            }
         }
 
         private bool ServerSupportsEai

src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpTransport.cs

@@ -20,6 +20,10 @@ internal sealed class SmtpTransport
         private readonly SmtpClient _client;
         private ICredentialsByHost? _credentials;
         private bool _shouldAbort;
+        // Written (set true) from property setters without holding the transport lock and read
+        // from IsConnected on the send path, so it is volatile to make an invalidating
+        // configuration change reliably observable across threads without widening locking.
+        private volatile bool _stale;
 
         private bool _enableSsl;
 
@@ -43,15 +47,19 @@ internal ICredentialsByHost? Credentials
             }
             set
             {
-                _credentials = value;
+                if (!ReferenceEquals(value, _credentials))
+                {
+                    _credentials = value;
+                    InvalidateCachedConnection();
+                }
             }
         }
 
         internal bool IsConnected
         {
             get
             {
-                return _connection != null && _connection.IsConnected;
+                return _connection != null && _connection.IsConnected && !_stale;
             }
         }
 
@@ -63,7 +71,11 @@ internal bool EnableSsl
             }
             set
             {
-                _enableSsl = value;
+                if (value != _enableSsl)
+                {
+                    _enableSsl = value;
+                    InvalidateCachedConnection();
+                }
             }
         }
 
@@ -89,6 +101,15 @@ internal Task GetConnectionAsync<TIOAdapter>(string host, int port, Cancellation
         {
             lock (this)
             {
+                // Abort any previously cached connection (for example one that became stale after a
+                // configuration change, or one whose connect attempt failed) so its socket is not
+                // leaked. Abort() only force-closes the socket without any network round-trip, so it
+                // is safe to run under the lock and does not block this async send path. A graceful
+                // QUIT is unnecessary for a connection we are discarding. Sends are serialized by
+                // SmtpClient._inCall, so no other GetConnectionAsync can run concurrently here.
+                _connection?.Abort();
+                _stale = false;
+
                 _connection = new SmtpConnection(this, _client, _credentials, _authenticationModules);
                 if (_shouldAbort)
                 {
@@ -147,6 +168,15 @@ internal void ReleaseConnection()
             _connection?.ReleaseConnection();
         }
 
+        // Marks any cached connection as stale without performing blocking work. The connection is
+        // aborted and replaced the next time one is established (see GetConnectionAsync). This is
+        // called when a property that affects how the connection is established (host, port,
+        // credentials, SSL settings, target name) changes.
+        internal void InvalidateCachedConnection()
+        {
+            _stale = true;
+        }
+
         internal void Abort()
         {
             lock (this)

src/libraries/System.Net.Mail/tests/Functional/SmtpClientConnectionTest.cs

@@ -9,6 +9,13 @@
 
 namespace System.Net.Mail.Tests
 {
+    public enum ConnectionAffectingProperty
+    {
+        Host,
+        Credentials,
+        TargetName,
+    }
+
     public abstract class SmtpClientConnectionTest<TSendMethod> : LoopbackServerTestBase<TSendMethod>
         where TSendMethod : ISendMethodProvider
     {
@@ -84,6 +91,77 @@ public async Task EHelloNotRecognized_RestartWithHello()
             await SendMail(new MailMessage("mono@novell.com", "everyone@novell.com", "introduction", "hello"));
             Assert.True(helloReceived, "HELO command was not received.");
         }
+
+        [Fact]
+        public async Task ChangingPort_DoesNotReuseConnectionToPreviousServer()
+        {
+            using LoopbackSmtpServer server2 = new LoopbackSmtpServer(Output);
+
+            await SendMail(new MailMessage("first@example.com", "everyone@novell.com", "introduction", "hello"));
+            Assert.Equal("<first@example.com>", Server.MailFrom);
+
+            // Point the client at a different server. The cached connection to the original
+            // server must be dropped so the next message is delivered to the new server.
+            Smtp.Port = server2.Port;
+
+            await SendMail(new MailMessage("second@example.com", "everyone@novell.com", "introduction", "hello"));
+
+            Assert.Equal("<second@example.com>", server2.MailFrom);
+            Assert.Equal(1, server2.ConnectionCount);
+
+            // The original server must not have received the second message.
+            Assert.Equal("<first@example.com>", Server.MailFrom);
+        }
+
+        [Theory]
+        [InlineData(ConnectionAffectingProperty.Host)]
+        [InlineData(ConnectionAffectingProperty.Credentials)]
+        [InlineData(ConnectionAffectingProperty.TargetName)]
+        public async Task ChangingConnectionProperty_EstablishesNewConnection(ConnectionAffectingProperty property)
+        {
+            Server.ReceiveMultipleConnections = true;
+
+            await SendMail(new MailMessage("first@example.com", "everyone@novell.com", "introduction", "hello"));
+            Assert.Equal(1, Server.ConnectionCount);
+            Assert.Equal("<first@example.com>", Server.MailFrom);
+
+            // Change a property that affects how the connection is established. The cached
+            // connection must be invalidated and a new one established on the next send.
+            switch (property)
+            {
+                case ConnectionAffectingProperty.Host:
+                    // A different value that still resolves to the loopback server. The default
+                    // TargetName should follow the host so authentication uses the correct SPN.
+                    Assert.Equal("SMTPSVC/localhost", Smtp.TargetName);
+                    Smtp.Host = "127.0.0.1";
+                    Assert.Equal("SMTPSVC/127.0.0.1", Smtp.TargetName);
+                    break;
+                case ConnectionAffectingProperty.Credentials:
+                    Smtp.Credentials = new NetworkCredential("foo", "bar");
+                    break;
+                case ConnectionAffectingProperty.TargetName:
+                    Smtp.TargetName = "SMTPSVC/example.com";
+                    break;
+            }
+
+            await SendMail(new MailMessage("second@example.com", "everyone@novell.com", "introduction", "hello"));
+            Assert.Equal(2, Server.ConnectionCount);
+            Assert.Equal("<second@example.com>", Server.MailFrom);
+        }
+
+        [Fact]
+        public async Task ChangingHost_PreservesExplicitlySetTargetName()
+        {
+            // A TargetName explicitly set by the caller must not be overwritten when the host
+            // changes, even though the default (host-derived) TargetName does follow the host.
+            Smtp.TargetName = "SMTPSVC/explicit.example.com";
+
+            await SendMail(new MailMessage("first@example.com", "everyone@novell.com", "introduction", "hello"));
+            Assert.Equal("SMTPSVC/explicit.example.com", Smtp.TargetName);
+
+            Smtp.Host = "127.0.0.1";
+            Assert.Equal("SMTPSVC/explicit.example.com", Smtp.TargetName);
+        }
     }
 
     public class SmtpClientConnectionTest_Send : SmtpClientConnectionTest<SyncSendMethod>

src/libraries/System.Net.Mail/tests/Functional/SmtpClientTlsTest.cs

@@ -185,6 +185,27 @@ public async Task ClientCertificateRequired_Sent()
             Assert.Equal(clientCert, receivedClientCert);
         }
 
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/120959", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot), nameof(PlatformDetection.IsAndroid))]
+        [Fact]
+        public async Task EnableSsl_ChangedAfterConnect_EstablishesNewEncryptedConnection()
+        {
+            Server.ReceiveMultipleConnections = true;
+            _serverCertValidationCallback = (cert, chain, errors) => true;
+
+            // First send happens over a plaintext connection.
+            await SendMail(new MailMessage("foo@example.com", "bar@example.com", "hello", "howdydoo"));
+            Assert.Equal(1, Server.ConnectionCount);
+            Assert.False(Server.IsEncrypted, "First connection should not be encrypted.");
+
+            // Enabling SSL must invalidate the cached plaintext connection so the next send
+            // establishes a new, encrypted connection instead of reusing the old one.
+            Smtp.EnableSsl = true;
+
+            await SendMail(new MailMessage("foo@example.com", "bar@example.com", "hello", "howdydoo"));
+            Assert.Equal(2, Server.ConnectionCount);
+            Assert.True(Server.IsEncrypted, "Second connection should be encrypted after enabling SSL.");
+        }
+
         private bool ServerCertValidationCallback(object sender, X509Certificate? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors)
         {
             if (_serverCertValidationCallback != null)