Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
<Compile Include="SQL\DataReaderTest\DataReaderStreamsTest.cs" />
<Compile Include="SQL\DataReaderTest\DataReaderTest.cs" />
<Compile Include="SQL\DataStreamTest\DataStreamTest.cs" />
<Compile Include="SQL\SqlStreamingXmlTest\SqlStreamingXmlTest.cs" />
<Compile Include="SQL\DateTimeTest\DateTimeTest.cs" />
<Compile Include="SQL\DNSCachingTest\DNSCachingTest.cs" />
<Compile Include="SQL\ExceptionTest\ConnectionExceptionTest.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Data;
using Xunit;

namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
public static class SqlStreamingXmlTest
{
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
public static void GetChars_NonAsciiContent()
{
SqlConnection connection = new(DataTestUtility.TCPConnectionString);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlConnection is created but not wrapped in a using statement. If an exception is thrown inside the using (SqlCommand ...) block before connection.Close() is called at line 36, the connection will never be disposed/closed, leaving a leaked connection. The established convention throughout the test suite is to declare SqlConnection with using, e.g. using SqlConnection connection = new(DataTestUtility.TCPConnectionString);. The same issue applies to all three test methods (lines 15, 43, and 70).

Copilot uses AI. Check for mistakes.
// XML containing non-ASCII characters:
// - \u00E9 (e-acute) - 2 bytes in UTF-8
// - \u00F1 (n-tilde) - 2 bytes in UTF-8
// - \u00FC (u-umlaut) - 2 bytes in UTF-8
string xml = "<r>caf\u00E9 se\u00F1or \u00FCber</r>";
int expectedLength = xml.Length;
string commandText = $"SELECT Convert(xml, N'{xml}')";

using (SqlCommand command = connection.CreateCommand())
{
connection.Open();
command.CommandText = commandText;

SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlDataReader returned by ExecuteReader is never disposed. If the assertions (or ReadAllChars) throw an exception, the reader is leaked. The established convention throughout the test suite is to wrap SqlDataReader in a using statement, e.g. using SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);. The same missing disposal applies to all three test methods (lines 29, 54, and 81).

Copilot uses AI. Check for mistakes.
Assert.True(sqlDataReader.Read(), "Expected to read a row");

(long length, string result) = ReadAllChars(sqlDataReader, expectedLength + 10);

Assert.Equal(expectedLength, length);
Assert.Equal(xml, result.Substring(0, (int)length));
connection.Close();
}
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
public static void GetChars_NonAsciiContent_BulkRead()
{
SqlConnection connection = new(DataTestUtility.TCPConnectionString);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlConnection is created but not wrapped in a using statement (same issue as in GetChars_NonAsciiContent). If an exception is thrown before connection.Close() at line 63, the connection is leaked. Use using SqlConnection connection = new(DataTestUtility.TCPConnectionString); instead.

Copilot uses AI. Check for mistakes.
// Same non-ASCII XML but read in a single bulk GetChars call
string xml = "<name>Jos\u00E9 Garc\u00EDa</name>";
int expectedLength = xml.Length;
string commandText = $"SELECT Convert(xml, N'{xml}')";

using (SqlCommand command = connection.CreateCommand())
{
connection.Open();
command.CommandText = commandText;

SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlDataReader is not disposed — same issue as GetChars_NonAsciiContent. Wrap it in a using statement to ensure proper resource cleanup regardless of test outcome.

Copilot uses AI. Check for mistakes.
Assert.True(sqlDataReader.Read(), "Expected to read a row");

char[] buffer = new char[expectedLength + 10];
long charsRead = sqlDataReader.GetChars(0, 0, buffer, 0, buffer.Length);

Assert.Equal(expectedLength, charsRead);
string result = new string(buffer, 0, (int)charsRead);
Assert.Equal(xml, result);
connection.Close();
}
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
public static void GetChars_CjkContent()
{
SqlConnection connection = new(DataTestUtility.TCPConnectionString);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlConnection is created but not wrapped in a using statement (same issue as in the other two tests). If an exception is thrown before connection.Close() at line 88, the connection is leaked. Use using SqlConnection connection = new(DataTestUtility.TCPConnectionString); instead.

Copilot uses AI. Check for mistakes.
// CJK characters: 3 bytes each in UTF-8
string xml = "<data>\u65E5\u672C\u8A9E\u30C6\u30B9\u30C8</data>";
int expectedLength = xml.Length;
string commandText = $"SELECT Convert(xml, N'{xml}')";

using (SqlCommand command = connection.CreateCommand())
{
connection.Open();
command.CommandText = commandText;

SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlDataReader is not disposed — same issue as GetChars_NonAsciiContent. Wrap it in a using statement to ensure proper resource cleanup regardless of test outcome.

Copilot uses AI. Check for mistakes.
Assert.True(sqlDataReader.Read(), "Expected to read a row");

(long length, string result) = ReadAllChars(sqlDataReader, expectedLength + 10);

Assert.Equal(expectedLength, length);
Assert.Equal(xml, result.Substring(0, (int)length));
connection.Close();
}
}

/// <summary>
/// Read all chars one at a time using GetChars with SequentialAccess,
/// replicating the pattern from issue #1877.
/// </summary>
private static (long, string) ReadAllChars(SqlDataReader sqlDataReader, int expectedSize)
{
char[] text = new char[expectedSize];
char[] buffer = new char[1];

long position = 0;
long numCharsRead;
do
{
numCharsRead = sqlDataReader.GetChars(0, position, buffer, 0, 1);
if (numCharsRead > 0)
{
text[position] = buffer[0];
position += numCharsRead;
Comment on lines +108 to +109
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text array in ReadAllChars is allocated with a fixed size (expectedSize). If the actual number of characters returned exceeds expectedSize, writing text[position] = buffer[0] will throw an IndexOutOfRangeException. While the + 10 buffer passed from the callers makes this unlikely for the specific test strings, there is no bounds check to protect against it. Consider either dynamically growing the buffer (e.g., using a List<char>) or adding a guard if (position < text.Length) before the array write.

Suggested change
text[position] = buffer[0];
position += numCharsRead;
if (position < text.Length)
{
text[position] = buffer[0];
position += numCharsRead;
}
else
{
// Prevent writing past the end of the fixed-size buffer.
break;
}

Copilot uses AI. Check for mistakes.
}
}
while (numCharsRead > 0);

return (position, new string(text));
}
}
}
Loading