From 407dfb413b9c3a9d4792c5992f54300d85637142 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 28 May 2025 22:05:13 +0100 Subject: [PATCH 1/3] fix sequential text reader bug and add covering test --- .../Data/SqlClient/SqlSequentialTextReader.cs | 2 +- .../SQL/DataReaderTest/DataReaderTest.cs | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs index a563d0dfc6..45f15c606f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs @@ -172,7 +172,7 @@ public override Task ReadAsync(char[] buffer, int index, int count) byte[] byteBuffer = PrepareByteBuffer(charsNeeded, out int byteBufferUsed); // Permit a 0 byte read in order to advance the reader to the correct column - if ((byteBufferUsed < byteBuffer.Length) || (byteBuffer.Length == 0)) + if ((byteBufferUsed <= byteBuffer.Length) || (byteBuffer.Length == 0)) { SqlDataReader reader = _reader; if (reader != null) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 9c4baf8050..020c0b296a 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -605,6 +605,56 @@ integrated into a comprehensive development } } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task CanReadSequentialDecreasingChunks() + { + const string baseString = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + StringBuilder inputBuilder = new StringBuilder(); + while (inputBuilder.Length < (64 * 1024)) + { + inputBuilder.Append(baseString); + inputBuilder.Append(' '); + } + + string input = inputBuilder.ToString(); + + StringBuilder resultBuilder = new StringBuilder(); + CancellationTokenSource cts = new CancellationTokenSource(); + using (var connection = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + await connection.OpenAsync(cts.Token); + + using (var command = connection.CreateCommand()) + { + command.CommandText = "SELECT CONVERT(varchar(max),@str) as a"; + command.Parameters.AddWithValue("@str", input); + + using (var reader = await command.ExecuteReaderAsync(CommandBehavior.SequentialAccess, cts.Token)) + { + if (await reader.ReadAsync(cts.Token)) + { + using (var textReader = reader.GetTextReader(0)) + { + var buffer = new char[4096]; + var charsReadCount = -1; + var start = 0; + while (charsReadCount != 0) + { + charsReadCount = await textReader.ReadAsync(buffer, start, buffer.Length - start); + resultBuilder.Append(buffer, start, charsReadCount); + start++; + } + } + } + } + } + } + + string result = resultBuilder.ToString(); + + Assert.Equal(input, result); + } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static async Task CanReadBinaryData() { From 5dce7ba503bd430ee90b85e20ea583938155c20a Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Fri, 30 May 2025 01:15:51 +0100 Subject: [PATCH 2/3] remove additional parens --- .../src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs index 45f15c606f..3ae05e0386 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSequentialTextReader.cs @@ -172,7 +172,7 @@ public override Task ReadAsync(char[] buffer, int index, int count) byte[] byteBuffer = PrepareByteBuffer(charsNeeded, out int byteBufferUsed); // Permit a 0 byte read in order to advance the reader to the correct column - if ((byteBufferUsed <= byteBuffer.Length) || (byteBuffer.Length == 0)) + if (byteBufferUsed <= byteBuffer.Length || byteBuffer.Length == 0) { SqlDataReader reader = _reader; if (reader != null) From d905a7bc3c505f63289e51af0b3601e49ce62ba5 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Mon, 2 Jun 2025 13:51:42 +0100 Subject: [PATCH 3/3] review feedback --- .../tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 020c0b296a..75f80e24a9 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -608,6 +608,10 @@ integrated into a comprehensive development [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static async Task CanReadSequentialDecreasingChunks() { + // pattern repeat input allows you to more easily identify if chunks are incorrectly + // related to each other by seeing the start and end of sequential chunks and checking + // if they correctly move to the next char while debugging + // simply repeating a single char can't tell you where in the string it went wrong. const string baseString = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; StringBuilder inputBuilder = new StringBuilder(); while (inputBuilder.Length < (64 * 1024))