Change TryReadString to use TryReadBytesWithContinue (#3422)

* change TryReadString to use TryReadBytesWithContinue

* review feedback

* tidy up sql

* Update src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs

---------

Co-authored-by: Cheena Malhotra <[email protected]>

Author: Wraith2

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

@@ -6374,7 +6374,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value,
                     }
                     else
                     {
-                        result = TryReadByteArrayWithContinue(stateObj, length, out b);
+                        result = stateObj.TryReadByteArrayWithContinue(length, out b);
                         if (result != TdsOperationStatus.Done)
                         {
                             return result;
@@ -6495,48 +6495,6 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value,
             return TdsOperationStatus.Done;
         }
 
-        private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject stateObj, int length, out byte[] bytes)
-        {
-            bytes = null;
-            int offset = 0;
-            byte[] temp = null;
-            (bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
-            if (canContinue)
-            {
-                if (isContinuing || isStarting)
-                {
-                    temp = stateObj.TryTakeSnapshotStorage() as byte[];
-                    Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length");
-                }
-                if (temp != null)
-                {
-                    offset = stateObj.GetSnapshotTotalSize();
-                }
-            }
-
-
-            if (temp == null)
-            {
-                temp = new byte[length];
-            }
-
-            TdsOperationStatus result = stateObj.TryReadByteArray(temp, length, out _, offset, isStarting || isContinuing);
-
-            if (result == TdsOperationStatus.Done)
-            {
-                bytes = temp;
-            }
-            else if (result == TdsOperationStatus.NeedMoreData)
-            {
-                if (isStarting || isContinuing)
-                {
-                    stateObj.SetSnapshotStorage(temp);
-                }
-            }
-
-            return result;
-        }
-
         private TdsOperationStatus TryReadSqlDateTime(SqlBuffer value, byte tdsType, int length, byte scale, TdsParserStateObject stateObj)
         {
             Span<byte> datetimeBuffer = ((uint)length <= 16) ? stackalloc byte[16] : new byte[length];

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

@@ -6570,7 +6570,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value,
                     }
                     else
                     {
-                        result = TryReadByteArrayWithContinue(stateObj, length, out b);
+                        result = stateObj.TryReadByteArrayWithContinue(length, out b);
                         if (result != TdsOperationStatus.Done)
                         {
                             return result;
@@ -6691,48 +6691,6 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value,
             return TdsOperationStatus.Done;
         }
 
-        private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject stateObj, int length, out byte[] bytes)
-        {
-            bytes = null;
-            int offset = 0;
-            byte[] temp = null;
-            (bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
-            if (canContinue)
-            {
-                if (isContinuing || isStarting)
-                {
-                    temp = stateObj.TryTakeSnapshotStorage() as byte[];
-                    Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length");
-                }
-                if (temp != null)
-                {
-                    offset = stateObj.GetSnapshotTotalSize();
-                }
-            }
-
-            
-            if (temp == null)
-            {
-                temp = new byte[length];
-            }
-
-            TdsOperationStatus result = stateObj.TryReadByteArray(temp, length, out _, offset, isStarting || isContinuing);
-
-            if (result == TdsOperationStatus.Done)
-            {
-                bytes = temp;
-            }
-            else if (result == TdsOperationStatus.NeedMoreData)
-            {
-                if (isStarting || isContinuing)
-                {
-                    stateObj.SetSnapshotStorage(temp);
-                }
-            }
-
-            return result;
-        }
-
         private TdsOperationStatus TryReadSqlDateTime(SqlBuffer value, byte tdsType, int length, byte scale, TdsParserStateObject stateObj)
         {
             Span<byte> datetimeBuffer = ((uint)length <= 16) ? stackalloc byte[16] : new byte[length];

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

@@ -1497,6 +1497,48 @@ public TdsOperationStatus TryReadByteArray(Span<byte> buff, int len, out int tot
             return TdsOperationStatus.Done;
         }
 
+        public TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] bytes)
+        {
+            bytes = null;
+            int offset = 0;
+            byte[] temp = null;
+            (bool canContinue, bool isStarting, bool isContinuing) = GetSnapshotStatuses();
+            if (canContinue)
+            {
+                if (isContinuing || isStarting)
+                {
+                    temp = TryTakeSnapshotStorage() as byte[];
+                    Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length");
+                }
+                if (temp != null)
+                {
+                    offset = GetSnapshotTotalSize();
+                }
+            }
+
+
+            if (temp == null)
+            {
+                temp = new byte[length];
+            }
+
+            TdsOperationStatus result = TryReadByteArray(temp, length, out _, offset, isStarting || isContinuing);
+
+            if (result == TdsOperationStatus.Done)
+            {
+                bytes = temp;
+            }
+            else if (result == TdsOperationStatus.NeedMoreData)
+            {
+                if (isStarting || isContinuing)
+                {
+                    SetSnapshotStorage(temp);
+                }
+            }
+
+            return result;
+        }
+
         // Takes no arguments and returns a byte from the buffer.  If the buffer is empty, it is filled
         // before the byte is returned.
         internal TdsOperationStatus TryReadByte(out byte value)
@@ -1843,21 +1885,13 @@ internal TdsOperationStatus TryReadString(int length, out string value)
 
             if (((_inBytesUsed + cBytes) > _inBytesRead) || (_inBytesPacket < cBytes))
             {
-                if (_bTmp == null || _bTmp.Length < cBytes)
-                {
-                    _bTmp = new byte[cBytes];
-                }
-
-                TdsOperationStatus result = TryReadByteArray(_bTmp, cBytes);
+                TdsOperationStatus result = TryReadByteArrayWithContinue(cBytes, out buf);
                 if (result != TdsOperationStatus.Done)
                 {
                     value = null;
                     return result;
                 }
 
-                // assign local to point to parser scratch buffer
-                buf = _bTmp;
-
                 AssertValidState();
             }
             else