Address Copilot review feedback

- ReturnValue: report actual valLen via unchecked((int)valLen) instead of
  int.MaxValue for more actionable error diagnostics
- TryReadSqlDateTime: use TdsEnums.MaxDateTimeLength constant instead of
  local const for consistency with other bounds checks
- SqlConnectionInternal: use ParsingErrorLength for session-recovery check
  to include the offending length in the error
- TdsEnums: broaden MaxTokenDataLength comment scope (not login-only),
  add MaxDateTimeLength constant
- FeatureExtAckBoundsTests: rewrite boundary test to inject token at
  exactly MaxTokenDataLength and assert bounds check does not fire
- TdsTokenBoundsTests: fix DateTime test comment (heap, not stack)

Author: paulmedynski

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs

@@ -1348,7 +1348,7 @@ internal void OnFeatureExtAck(int featureId, byte[] data)
 
                         if (len < 0 || len > data.Length - i)
                         {
-                            throw SQL.ParsingError(ParsingErrorState.CorruptedTdsStream);
+                            throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, len);
                         }
 
                         byte[] stateData = new byte[len];

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs

@@ -84,14 +84,17 @@ internal static class TdsEnums
         public const int MAX_PACKET_SIZE = 32768;
         public const int MAX_SERVER_USER_NAME = 256;  // obtained from luxor
 
-        // Maximum allowed data length for login-phase token payloads (feature ext ack,
+        // Maximum allowed data length for token payloads (feature ext ack,
         // session state, fedauth info). Prevents a malicious server from causing
-        // unbounded memory allocation on the client before authentication completes.
+        // unbounded memory allocation via spoofed token length fields.
         internal const int MaxTokenDataLength = 1 << 20; // 1 MB
 
         // Maximum allowed data length for a DTC promote transaction propagation token.
         internal const int MaxPromoteTransactionLength = 1 << 16; // 64 KB
 
+        // Maximum valid wire size for datetime types (DateTimeOffset = 5 time + 3 date + 2 offset).
+        internal const int MaxDateTimeLength = 10;
+
         // Severity  0 - 10 indicates informational (non-error) messages
         // Severity 11 - 16 indicates errors that can be corrected by user (syntax errors, etc...)
         // Severity 17 - 19 indicates failure due to insufficient resources in the server

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

@@ -4940,7 +4940,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length,
             }
             else if (valLen > (ulong)int.MaxValue)
             {
-                throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, int.MaxValue);
+                throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, unchecked((int)valLen));
             }
             else
             {
@@ -7141,13 +7141,12 @@ private TdsOperationStatus TryReadSqlDateTime(SqlBuffer value, byte tdsType, int
         {
             // DateTimeOffset is the largest datetime type at 10 bytes (5 time + 3 date + 2 offset).
             // Reject anything larger to prevent heap allocation from spoofed metadata.
-            const int MaxDateTimeLength = 10;
-            if (length < 0 || length > MaxDateTimeLength)
+            if (length < 0 || length > TdsEnums.MaxDateTimeLength)
             {
                 throw SQL.ParsingErrorLength(ParsingErrorState.CorruptedTdsStream, length);
             }
 
-            Span<byte> datetimeBuffer = stackalloc byte[MaxDateTimeLength];
+            Span<byte> datetimeBuffer = stackalloc byte[TdsEnums.MaxDateTimeLength];
 
             TdsOperationStatus result = stateObj.TryReadByteArray(datetimeBuffer, length);
             if (result != TdsOperationStatus.Done)

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/FeatureExtAckBoundsTests.cs

@@ -78,44 +78,50 @@ public void FeatureExtAck_OversizedDataLength_ThrowsParsingError()
     }
 
     /// <summary>
-    /// Verifies that a FeatureExtAck token with a data length at or below the
+    /// Verifies that a FeatureExtAck token with a data length at exactly the
     /// allowed maximum is accepted without error, confirming there is no
     /// off-by-one in the bounds check.
     /// </summary>
     [Fact]
     public void FeatureExtAck_MaxAllowedDataLength_DoesNotThrow()
     {
-        // Arrange: inject a FeatureExtAck token at exactly the maximum allowed size.
-        // We use a small real payload to avoid actually allocating 1 MB in the test;
-        // the server will write a data length that equals the actual data length.
-        // This verifies that the boundary value is accepted (not off-by-one).
-        byte[] legitimateData = new byte[] { 0x01 }; // 1-byte GlobalTransactions ack
-
+        // Arrange: inject a FeatureExtAck token whose declared data length equals
+        // MaxTokenDataLength exactly. The bounds check should NOT fire for this
+        // value. The connection will fail for other reasons (not enough data on
+        // the wire), but the error must NOT be state 18 (CorruptedTdsStream).
         _server.OnAuthenticationResponseCompleted = responseMessage =>
         {
-            // Find existing FeatureExtAck or create one
             var existing = responseMessage.OfType<TDSFeatureExtAckToken>().FirstOrDefault();
-            if (existing == null)
+            if (existing != null)
             {
-                int doneIndex = responseMessage.FindIndex(t => t is TDSDoneToken);
-                if (doneIndex < 0)
-                {
-                    doneIndex = responseMessage.Count;
-                }
-
-                var token = new TDSFeatureExtAckToken(
-                    new TDSFeatureExtAckGenericOption(
-                        (TDSFeatureID)TdsEnums.FEATUREEXT_GLOBALTRANSACTIONS,
-                        (uint)legitimateData.Length,
-                        legitimateData));
-                responseMessage.Insert(doneIndex, token);
+                responseMessage.Remove(existing);
+            }
+
+            int doneIndex = responseMessage.FindIndex(t => t is TDSDoneToken);
+            if (doneIndex < 0)
+            {
+                doneIndex = responseMessage.Count;
             }
+
+            // Insert token with dataLen = MaxTokenDataLength (at boundary, not over)
+            responseMessage.Insert(doneIndex, new MaliciousFeatureExtAckToken(
+                featureId: (TDSFeatureID)TdsEnums.FEATUREEXT_GLOBALTRANSACTIONS,
+                claimedDataLen: (uint)TdsEnums.MaxTokenDataLength));
         };
 
-        // Act & Assert: connection should open successfully
-        using SqlConnection connection = new(_connectionString);
-        connection.Open();
-        Assert.Equal(System.Data.ConnectionState.Open, connection.State);
+        try
+        {
+            using SqlConnection connection = new(_connectionString);
+            // The connection will fail (insufficient data for the claimed length),
+            // but the failure must NOT be the bounds check (state 18 with MaxTokenDataLength+1).
+            Exception ex = Assert.ThrowsAny<Exception>(connection.Open);
+            // Confirm it's NOT the bounds-check error (which would report MaxTokenDataLength+1)
+            Assert.DoesNotContain((TdsEnums.MaxTokenDataLength + 1).ToString(), ex.Message);
+        }
+        finally
+        {
+            _server.OnAuthenticationResponseCompleted = null;
+        }
     }
 
     /// <summary>

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/TdsTokenBoundsTests.cs

@@ -507,7 +507,7 @@ public void BatchResponse_PromoteTransaction_OversizedLength_ThrowsParsingError(
     /// Verifies that <c>TryReadSqlDateTime</c> rejects a TIME column value whose
     /// data length byte exceeds the maximum datetime wire size (10 bytes). A
     /// malicious server could set this length to a large value, causing the
-    /// parser to attempt an oversized stackalloc.
+    /// parser to attempt an unbounded heap allocation.
     /// </summary>
     [Fact]
     public void BatchResponse_DateTime_OversizedLength_ThrowsParsingError()