Revert "Fix rare multi-packet string corruption (#3505) (#3513)"

samsharma2700 · samsharma2700 · commit b06cb5ba37f6 · 2025-08-05T14:19:45.000-07:00
This reverts commit 1af7327.
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
@@ -13085,10 +13085,11 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
 
             if (canContinue)
             {
-                temp = stateObj.TryTakeSnapshotStorage() as char[];
-                Debug.Assert(temp != null || !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");
-                Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length");
-                
+                if (isContinuing || isStarting)
+                {
+                    temp = stateObj.TryTakeSnapshotStorage() as char[];
+                    Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length");
+                }
                 if (temp != null)
                 {
                     startOffset = stateObj.GetSnapshotTotalSize();
@@ -13104,7 +13105,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
                 supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot
                 rentedBuff: ref buffIsRented,
                 startOffset,
-                canContinue
+                isStarting || isContinuing
             );
 
             if (result == TdsOperationStatus.Done)
@@ -13131,7 +13132,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
             }
             else if (result == TdsOperationStatus.NeedMoreData)
             {
-                if (canContinue)
+                if (isStarting || isContinuing)
                 {
                     stateObj.SetSnapshotStorage(temp);
                 }
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
@@ -13266,10 +13266,11 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
 
             if (canContinue)
             {
-                temp = stateObj.TryTakeSnapshotStorage() as char[];
-                Debug.Assert(temp != null || !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");
-                Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length");
-
+                if (isContinuing || isStarting)
+                {
+                    temp = stateObj.TryTakeSnapshotStorage() as char[];
+                    Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length");
+                }
                 if (temp != null)
                 {
                     startOffset = stateObj.GetSnapshotTotalSize();
@@ -13284,8 +13285,8 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
                 out length, 
                 supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot
                 rentedBuff: ref buffIsRented, 
-                startOffset,
-                canContinue
+                startOffset, 
+                isStarting || isContinuing
             );
 
             if (result == TdsOperationStatus.Done)
@@ -13312,7 +13313,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
             }
             else if (result == TdsOperationStatus.NeedMoreData)
             {
-                if (canContinue)
+                if (isStarting || isContinuing)
                 {
                     stateObj.SetSnapshotStorage(temp);
                 }
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
@@ -1527,10 +1527,11 @@ public TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] by
             (bool canContinue, bool isStarting, bool isContinuing) = GetSnapshotStatuses();
             if (canContinue)
             {
-                temp = TryTakeSnapshotStorage() as byte[];
-                Debug.Assert(temp != null || !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");
-                Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length");
-                
+                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();
@@ -1551,7 +1552,7 @@ public TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] by
             }
             else if (result == TdsOperationStatus.NeedMoreData)
             {
-                if (canContinue)
+                if (isStarting || isContinuing)
                 {
                     SetSnapshotStorage(temp);
                 }
@@ -1980,10 +1981,11 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En
                     int startOffset = 0;
                     if (canContinue)
                     {
-                        buf = TryTakeSnapshotStorage() as byte[];
-                        Debug.Assert(buf != null || !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");
-                        Debug.Assert(buf == null || buf.Length == length, "stored buffer length must be null or must have been created with the correct length");
-                        
+                        if (isContinuing || isStarting)
+                        {
+                            buf = TryTakeSnapshotStorage() as byte[];
+                            Debug.Assert(buf == null || buf.Length == length, "stored buffer length must be null or must have been created with the correct length");
+                        }
                         if (buf != null)
                         {
                             startOffset = GetSnapshotTotalSize();
@@ -2001,7 +2003,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En
                     {
                         if (result == TdsOperationStatus.NeedMoreData)
                         {
-                            if (canContinue)
+                            if (isStarting || isContinuing)
                             {
                                 SetSnapshotStorage(buf);
                             }
diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs

Original file line number	Diff line number	Diff line change
`@@ -13085,10 +13085,11 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj`
`13085`	`13085`
`13086`	`13086`	`if (canContinue)`
`13087`	`13087`	`{`
`13088`		`- temp = stateObj.TryTakeSnapshotStorage() as char[];`
`13089`		`- Debug.Assert(temp != null \|\| !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");`
`13090`		`- Debug.Assert(temp == null \|\| length == int.MaxValue \|\| temp.Length == length, "stored buffer length must be null or must have been created with the correct length");`
`13091`		`-`
	`13088`	`+ if (isContinuing \|\| isStarting)`
	`13089`	`+ {`
	`13090`	`+ temp = stateObj.TryTakeSnapshotStorage() as char[];`
	`13091`	`+ Debug.Assert(temp == null \|\| length == int.MaxValue \|\| temp.Length == length, "stored buffer length must be null or must have been created with the correct length");`
	`13092`	`+ }`
`13092`	`13093`	`if (temp != null)`
`13093`	`13094`	`{`
`13094`	`13095`	`startOffset = stateObj.GetSnapshotTotalSize();`
`@@ -13104,7 +13105,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj`
`13104`	`13105`	`supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot`
`13105`	`13106`	`rentedBuff: ref buffIsRented,`
`13106`	`13107`	`startOffset,`
`13107`		`- canContinue`
	`13108`	`+ isStarting \|\| isContinuing`
`13108`	`13109`	`);`
`13109`	`13110`
`13110`	`13111`	`if (result == TdsOperationStatus.Done)`
`@@ -13131,7 +13132,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj`
`13131`	`13132`	`}`
`13132`	`13133`	`else if (result == TdsOperationStatus.NeedMoreData)`
`13133`	`13134`	`{`
`13134`		`- if (canContinue)`
	`13135`	`+ if (isStarting \|\| isContinuing)`
`13135`	`13136`	`{`
`13136`	`13137`	`stateObj.SetSnapshotStorage(temp);`
`13137`	`13138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13266,10 +13266,11 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj`
`13266`	`13266`
`13267`	`13267`	`if (canContinue)`
`13268`	`13268`	`{`
`13269`		`- temp = stateObj.TryTakeSnapshotStorage() as char[];`
`13270`		`- Debug.Assert(temp != null \|\| !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");`
`13271`		`- Debug.Assert(temp == null \|\| length == int.MaxValue \|\| temp.Length == length, "stored buffer length must be null or must have been created with the correct length");`
`13272`		`-`
	`13269`	`+ if (isContinuing \|\| isStarting)`
	`13270`	`+ {`
	`13271`	`+ temp = stateObj.TryTakeSnapshotStorage() as char[];`
	`13272`	`+ Debug.Assert(temp == null \|\| length == int.MaxValue \|\| temp.Length == length, "stored buffer length must be null or must have been created with the correct length");`
	`13273`	`+ }`
`13273`	`13274`	`if (temp != null)`
`13274`	`13275`	`{`
`13275`	`13276`	`startOffset = stateObj.GetSnapshotTotalSize();`
`@@ -13284,8 +13285,8 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj`
`13284`	`13285`	`out length,`
`13285`	`13286`	`supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot`
`13286`	`13287`	`rentedBuff: ref buffIsRented,`
`13287`		`- startOffset,`
`13288`		`- canContinue`
	`13288`	`+ startOffset,`
	`13289`	`+ isStarting \|\| isContinuing`
`13289`	`13290`	`);`
`13290`	`13291`
`13291`	`13292`	`if (result == TdsOperationStatus.Done)`
`@@ -13312,7 +13313,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj`
`13312`	`13313`	`}`
`13313`	`13314`	`else if (result == TdsOperationStatus.NeedMoreData)`
`13314`	`13315`	`{`
`13315`		`- if (canContinue)`
	`13316`	`+ if (isStarting \|\| isContinuing)`
`13316`	`13317`	`{`
`13317`	`13318`	`stateObj.SetSnapshotStorage(temp);`
`13318`	`13319`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1527,10 +1527,11 @@ public TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] by`
`1527`	`1527`	`(bool canContinue, bool isStarting, bool isContinuing) = GetSnapshotStatuses();`
`1528`	`1528`	`if (canContinue)`
`1529`	`1529`	`{`
`1530`		`- temp = TryTakeSnapshotStorage() as byte[];`
`1531`		`- Debug.Assert(temp != null \|\| !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");`
`1532`		`- Debug.Assert(bytes == null \|\| bytes.Length == length, "stored buffer length must be null or must have been created with the correct length");`
`1533`		`-`
	`1530`	`+ if (isContinuing \|\| isStarting)`
	`1531`	`+ {`
	`1532`	`+ temp = TryTakeSnapshotStorage() as byte[];`
	`1533`	`+ Debug.Assert(bytes == null \|\| bytes.Length == length, "stored buffer length must be null or must have been created with the correct length");`
	`1534`	`+ }`
`1534`	`1535`	`if (temp != null)`
`1535`	`1536`	`{`
`1536`	`1537`	`offset = GetSnapshotTotalSize();`
`@@ -1551,7 +1552,7 @@ public TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] by`
`1551`	`1552`	`}`
`1552`	`1553`	`else if (result == TdsOperationStatus.NeedMoreData)`
`1553`	`1554`	`{`
`1554`		`- if (canContinue)`
	`1555`	`+ if (isStarting \|\| isContinuing)`
`1555`	`1556`	`{`
`1556`	`1557`	`SetSnapshotStorage(temp);`
`1557`	`1558`	`}`
`@@ -1980,10 +1981,11 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En`
`1980`	`1981`	`int startOffset = 0;`
`1981`	`1982`	`if (canContinue)`
`1982`	`1983`	`{`
`1983`		`- buf = TryTakeSnapshotStorage() as byte[];`
`1984`		`- Debug.Assert(buf != null \|\| !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from");`
`1985`		`- Debug.Assert(buf == null \|\| buf.Length == length, "stored buffer length must be null or must have been created with the correct length");`
`1986`		`-`
	`1984`	`+ if (isContinuing \|\| isStarting)`
	`1985`	`+ {`
	`1986`	`+ buf = TryTakeSnapshotStorage() as byte[];`
	`1987`	`+ Debug.Assert(buf == null \|\| buf.Length == length, "stored buffer length must be null or must have been created with the correct length");`
	`1988`	`+ }`
`1987`	`1989`	`if (buf != null)`
`1988`	`1990`	`{`
`1989`	`1991`	`startOffset = GetSnapshotTotalSize();`
`@@ -2001,7 +2003,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En`
`2001`	`2003`	`{`
`2002`	`2004`	`if (result == TdsOperationStatus.NeedMoreData)`
`2003`	`2005`	`{`
`2004`		`- if (canContinue)`
	`2006`	`+ if (isStarting \|\| isContinuing)`
`2005`	`2007`	`{`
`2006`	`2008`	`SetSnapshotStorage(buf);`
`2007`	`2009`	`}`