Fix MemoryStream to allow appending when GetChars request spans multiple elements

jimhblythe · jimhblythe · commit e2dec42ec756 · 2026-02-26T16:14:11.000-08:00
Enhance comments within SqlStreamingXml
Extend Manual tests to fully cover GetChars
WriteXmlElement includes uncovered paths not accessible for SQL XML column types which normalize Whitespace, CDATA, EntityReference, XmlDeclaration, ProcessingInstruction, DocumentType, and Comment node types
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlStream.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlStream.cs
@@ -477,7 +477,12 @@ private long TotalLength
 
     sealed internal class SqlStreamingXml
     {
-        private static readonly XmlWriterSettings s_writerSettings = new() { CloseOutput = true, ConformanceLevel = ConformanceLevel.Fragment, Encoding = new UTF8Encoding(false) };
+        private static readonly XmlWriterSettings s_writerSettings = new() {
+            CloseOutput = true,
+            ConformanceLevel = ConformanceLevel.Fragment,
+            // Potentially limits XML to not supporting UTF-16 characters, but this is required to avoid writing
+            // a byte order mark and is consistent with prior default used within StringWriter/StringBuilder.
+            Encoding = new UTF8Encoding(false) };
 
         private readonly int _columnOrdinal;
         private SqlDataReader _reader;
@@ -525,7 +530,8 @@ public long GetChars(long dataIndex, char[] buffer, int bufferIndex, int length)
             }
             else if (dataIndex > _charsRemoved)
             {
-                charsToSkip = (int)(dataIndex - _charsRemoved);
+                //dataIndex is zero-based, but _charsRemoved is one-based, so the difference is the number of chars to skip in the MemoryStream before we start copying data to the buffer
+                charsToSkip = dataIndex - _charsRemoved;
             }
 
             // If buffer parameter is null, we have to return -1 since there is no way for us to know the
@@ -550,7 +556,7 @@ public long GetChars(long dataIndex, char[] buffer, int bufferIndex, int length)
                 //_xmlWriter.WriteNode(_xmlReader, true);
                 //  _xmlWriter.Flush();
                 WriteXmlElement();
-                // Update memoryStreamRemaining based on the number of chars just written to the MemoryStream
+                // Update memoryStreamRemaining based on the number of bytes/chars just written to the MemoryStream
                 memoryStreamRemaining = _memoryStream.Length - _memoryStream.Position;
                 if (charsToSkip > 0)
                 {
@@ -583,6 +589,7 @@ public long GetChars(long dataIndex, char[] buffer, int bufferIndex, int length)
             cnt = memoryStreamRemaining < length ? memoryStreamRemaining : length;
             for (int i = 0; i < cnt; i++)
             {
+                // ReadByte moves the Position forward
                 buffer[bufferIndex + i] = (char)_memoryStream.ReadByte();
             }
             _charsRemoved += cnt;
@@ -598,10 +605,15 @@ private void WriteXmlElement()
             const int WriteNodeBufferSize = 1024;
 
             long memoryStreamPosition = _memoryStream.Position;
+            // Move the Position to the end of the MemoryStream since we are always appending.
+            _memoryStream.Seek(0, SeekOrigin.End);
 
             _xmlReader.Read();
             switch (_xmlReader.NodeType)
             {
+                // Note: Whitespace, CDATA, EntityReference, XmlDeclaration, ProcessingInstruction, DocumentType, and Comment node types
+                // are not expected in the XML returned from SQL Server as it normalizes them out, but handle them just in case.
+                // SignificantWhitespace will occur when used with xml:space="preserve"
                 case XmlNodeType.Element:
                     _xmlWriter.WriteStartElement(_xmlReader.Prefix, _xmlReader.LocalName, _xmlReader.NamespaceURI);
                     _xmlWriter.WriteAttributes(_xmlReader, true);
@@ -651,6 +663,7 @@ private void WriteXmlElement()
                     break;
             }
             _xmlWriter.Flush();
+            // Reset the Position back to where it was before writing this element so that the caller can continue reading from the expected position.
             _memoryStream.Position = memoryStreamPosition;
         }
     }
diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlStreamingXmlTest/SqlStreamingXmlTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlStreamingXmlTest/SqlStreamingXmlTest.cs
@@ -6,17 +6,17 @@
 using System.Data;
 using System.Diagnostics;
 using System.Globalization;
+using System.Xml.Linq;
 using Xunit;
 
 namespace Microsoft.Data.SqlClient.ManualTesting.Tests
 {
     public static class SqlStreamingXmlTest
     {
         [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
-        public static void LinearSingleNode()
+        public static void Linear_SingleNode()
         {
-            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
-            // Use a literal XML column of the specified size. The XML is constructed by replicating a string of 'B' characters to reach the desired size, and wrapping it in XML tags.
+            // Use literal XML column constructed by replicating a string of 'B' characters to reach the desired size, and wrapping it in XML tags.
             const string commandTextBase = "SELECT Convert(xml, N'<foo>' + REPLICATE(CAST('' AS nvarchar(max)) +N'B', ({0} * 1024 * 1024) - 11) + N'</foo>')";
 
             TimeSpan time1 = TimedExecution(commandTextBase, 1);
@@ -27,10 +27,9 @@ public static void LinearSingleNode()
         }
 
         [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
-        public static void LinearMultipleNodes()
+        public static void Linear_MultipleNodes()
         {
-            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
-            // Use a literal XML column with the specified number of 1MB elements. The XML is constructed by replicating a string of 'B' characters to reach 1MB, then replicating to the desired number of elements.
+            // Use literal XML column constructed by replicating a string of 'B' characters to reach 1MB, then replicating to the desired number of elements.
             const string commandTextBase = "SELECT Convert(xml, REPLICATE(N'<foo>' + REPLICATE(CAST('' AS nvarchar(max)) + N'B', (1024 * 1024) - 11) + N'</foo>', {0}))";
 
             TimeSpan time1 = TimedExecution(commandTextBase, 1);
@@ -40,10 +39,244 @@ public static void LinearMultipleNodes()
             Assert.True(time5.TotalMilliseconds <= (time1.TotalMilliseconds * 6), $"Execution time did not follow linear scale: 1x={time1.TotalMilliseconds}ms vs. 5x={time5.TotalMilliseconds}ms");
         }
 
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static void GetChars_RequiresBuffer()
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            const string commandText = "SELECT Convert(xml, N'<foo>bar</foo>')";
+            long charCount = 0;
+
+            using (SqlCommand command = connection.CreateCommand())
+            {
+                connection.Open();
+                command.CommandText = commandText;
+
+                SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
+                if (sqlDataReader.Read())
+                {
+                    charCount = sqlDataReader.GetChars(0, 0, null, 0, 1);
+                }
+                connection.Close();
+            }
+
+            //verify -1 is returned since buffer was not provided
+            Assert.Equal(-1, charCount);
+        }
+
+        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        [InlineData(true)]
+        [InlineData(false)]
+        public static void GetChars_SequentialDataIndex(bool backwards)
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            const string commandText = "SELECT Convert(xml, N'<foo>bar</foo>')";
+            char[] buffer = new char[2];
+
+            using (SqlCommand command = connection.CreateCommand())
+            {
+                connection.Open();
+                command.CommandText = commandText;
+
+                SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
+                if (sqlDataReader.Read())
+                {
+                    sqlDataReader.GetChars(0, 0, buffer, 0, 2);
+                    // Verify that providing the same or lower index than the previous call results in an exception.
+                    // When backwards is true we test providing an index that is one less than the previous call,
+                    // otherwise we test providing the same index as the previous call - both should not be allowed.
+                    Assert.Throws<InvalidOperationException>(() => sqlDataReader.GetChars(0, backwards ? 0 : 1, buffer, 0, 2));
+                }
+                connection.Close();
+            }
+        }
+
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static void GetChars_PartialSingleElement()
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            const string commandText = "SELECT Convert(xml, N'<foo>_bar_baz</foo>')";
+            long charCount = 0;
+            char[] buffer = new char[3];
+
+            using (SqlCommand command = connection.CreateCommand())
+            {
+                connection.Open();
+                command.CommandText = commandText;
+
+                SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
+                if (sqlDataReader.Read())
+                {
+                    // Read just the 'bar' characters from the XML by specifying the offset, and the length of 3.
+                    // The offset is 6 to skip the entire first element '<foo>' and the initial '_' part of text.
+                    charCount = sqlDataReader.GetChars(0, 6, buffer, 0, 3);
+                }
+                connection.Close();
+            }
+
+            Assert.Equal(3, charCount);
+            Assert.Equal("bar", new string(buffer));
+        }
+
+        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        [InlineData(true)]
+        [InlineData(false)]
+        public static void GetChars_PartialAcrossElements(bool initialRead)
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            const string commandText = "SELECT Convert(xml, N'<foobar>baz</foobar>')";
+            long charCount = 0;
+            char[] buffer = new char[8];
+
+            using (SqlCommand command = connection.CreateCommand())
+            {
+                connection.Open();
+                command.CommandText = commandText;
+
+                SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
+                if (sqlDataReader.Read())
+                {
+                    if (initialRead)
+                    {
+                        // When initialRead is true, we verify continuation after a previous read,
+                        // otherwise we just verify that we can read across XML elements in a single call.
+                        char[] initialBuffer = new char[2];
+                        sqlDataReader.GetChars(0, 0, initialBuffer, 0, 2);
+                        Assert.Equal("<f", new string(initialBuffer));
+                        // Verify skipping within the existing initial element.
+                        sqlDataReader.GetChars(0, 3, initialBuffer, 0, 2);
+                        Assert.Equal("ob", new string(initialBuffer));
+                    }
+                    // Read the 'r>baz</f' characters across XML elements by specifying the offset, and the length of 8.
+                    // The offset is 6 to skip the '<fooba' characters.
+                    charCount = sqlDataReader.GetChars(0, 6, buffer, 0, 8);
+                }
+                connection.Close();
+            }
+
+            Assert.Equal(8, charCount);
+            Assert.Equal("r>baz</f", new string(buffer));
+        }
+
+        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        [InlineData(true)]
+        [InlineData(false)]
+        public static void GetChars_ExcessiveLength(bool initialRead)
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            string xml = """<foo>_bar_baz</foo>""";
+            int expectedSize = 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);
+                if (sqlDataReader.Read())
+                {
+                    if (initialRead)
+                    {
+                        // When initialRead is true, we verify continuation after a previous read,
+                        // otherwise we just verify that we can read everything in a single call.
+                        char[] initialBuffer = new char[2];
+                        long initialLength = sqlDataReader.GetChars(0, 0, initialBuffer, 0, 2);
+                        char[] remainingBuffer = new char[98];
+                        long remainingLength = sqlDataReader.GetChars(0, 2, remainingBuffer, 0, 98);
+                        string combined = new string(initialBuffer) + new string(remainingBuffer);
+
+                        Assert.Equal(expectedSize, initialLength + remainingLength);
+                        Assert.Equal(xml, combined.Substring(0, expectedSize));
+                    }
+                    else
+                    {
+                        // Try to read more characters than the actual XML to verify that the method returns only the actual number of characters.
+                        (long length, string text) = ReadAllChars(sqlDataReader, 100);
+
+                        Assert.Equal(expectedSize, length);
+                        Assert.Equal(xml, text.Substring(0, expectedSize));
+                    }
+                }
+                connection.Close();
+            }
+        }
+
+        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        [InlineData(true)]
+        [InlineData(false)]
+        public static void GetChars_ExcessiveDataIndex(bool initialRead)
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            string xml = """<foo>_bar_baz</foo>""";
+            string commandText = $"SELECT Convert(xml, N'{xml}')";
+
+            using (SqlCommand command = connection.CreateCommand())
+            {
+                connection.Open();
+                command.CommandText = commandText;
+
+                SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);
+                if (sqlDataReader.Read())
+                {
+                    if (initialRead)
+                    {
+                        // When initialRead is true, we verify continuation after a previous read,
+                        // otherwise we just verify the large DataIndex in a single call.
+                        char[] initialBuffer = new char[2];
+                        long initialLength = sqlDataReader.GetChars(0, 0, initialBuffer, 0, 2);
+                        Assert.Equal(2, initialLength);
+                    }
+
+                    // buffer will not be touched since the DataIndex is beyond the end of the XML, but a suitable buffer must still be provided.
+                    char[] buffer = new char[100];
+                    long length = sqlDataReader.GetChars(0, 100, buffer, 0, 2);
+                    Assert.Equal(0, length);
+                }
+                connection.Close();
+            }
+        }
+
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
+        public static void GetChars_AsXDocument()
+        {
+            SqlConnection connection = new(DataTestUtility.TCPConnectionString);
+            // Use a more complex XML column verify through XDocument.
+            string xml = """<Person Id="1" Role="Admin"><Name>John</Name><Children /><PreservedWhitespace xml:space="preserve"> </PreservedWhitespace></Person>""";
+            XDocument expect = XDocument.Parse(xml);
+            int expectedSize = 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);
+                if (sqlDataReader.Read())
+                {
+                    (long length, string xmlString) = ReadAllChars(sqlDataReader, expectedSize);
+
+                    Assert.Equal(expectedSize, length);
+                    XDocument actual = XDocument.Parse(xmlString);
+                    Assert.Equal((int)expect.Root.Attribute("Id"), (int)actual.Root.Attribute("Id"));
+                    Assert.Equal((string)expect.Root.Attribute("Role"), (string)actual.Root.Attribute("Role"));
+                    Assert.NotNull(expect.Root.Element("Name")?.Value);
+                    Assert.Equal(expect.Root.Element("Name")!.Value, actual.Root.Element("Name")!.Value);
+                    Assert.NotNull(expect.Root.Element("Children")?.HasElements);
+                    Assert.Equal(expect.Root.Element("Children")!.HasElements, actual.Root.Element("Children")?.HasElements);
+                    Assert.NotNull(expect.Root.Element("PreservedWhitespace")?.Value);
+                    Assert.Equal(expect.Root.Element("PreservedWhitespace")!.Value, actual.Root.Element("PreservedWhitespace")!.Value);
+                }
+                connection.Close();
+            }
+        }
+
         private static TimeSpan TimedExecution(string commandTextBase, int scale)
         {
             SqlConnection connection = new(DataTestUtility.TCPConnectionString);
-            var stopwatch = new Stopwatch();
+            Stopwatch stopwatch = new Stopwatch();
+            int expectedSize = scale * 1024 * 1024;
+
 
             using (SqlCommand command = connection.CreateCommand())
             {
@@ -54,8 +287,9 @@ private static TimeSpan TimedExecution(string commandTextBase, int scale)
                 if (sqlDataReader.Read())
                 {
                     stopwatch.Start();
-                    ReadAllChars(sqlDataReader, scale);
+                    (long length, string _) = ReadAllChars(sqlDataReader, expectedSize);
                     stopwatch.Stop();
+                    Assert.Equal(expectedSize, length);
                 }
                 connection.Close();
             }
@@ -66,11 +300,10 @@ private static TimeSpan TimedExecution(string commandTextBase, int scale)
         /// <summary>
         /// Replicate the reading approach used with issue #1877
         /// </summary>
-        private static void ReadAllChars(SqlDataReader sqlDataReader, int expectedMB)
+        private static (long, string) ReadAllChars(SqlDataReader sqlDataReader, int expectedSize)
         {
-            var expectedSize = expectedMB * 1024 * 1024;
-            var text = new char[expectedSize];
-            var buffer = new char[1];
+            char[] text = new char[expectedSize];
+            char[] buffer = new char[1];
 
             long position = 0;
             long numCharsRead;
@@ -85,7 +318,7 @@ private static void ReadAllChars(SqlDataReader sqlDataReader, int expectedMB)
             }
             while (numCharsRead > 0);
 
-            Assert.Equal(expectedSize, position);
+            return (position, new string(text));
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -477,7 +477,12 @@ private long TotalLength`
`477`	`477`
`478`	`478`	`sealed internal class SqlStreamingXml`
`479`	`479`	`{`
`480`		`- private static readonly XmlWriterSettings s_writerSettings = new() { CloseOutput = true, ConformanceLevel = ConformanceLevel.Fragment, Encoding = new UTF8Encoding(false) };`
	`480`	`+ private static readonly XmlWriterSettings s_writerSettings = new() {`
	`481`	`+ CloseOutput = true,`
	`482`	`+ ConformanceLevel = ConformanceLevel.Fragment,`
	`483`	`+ // Potentially limits XML to not supporting UTF-16 characters, but this is required to avoid writing`
	`484`	`+ // a byte order mark and is consistent with prior default used within StringWriter/StringBuilder.`
	`485`	`+ Encoding = new UTF8Encoding(false) };`
`481`	`486`
`482`	`487`	`private readonly int _columnOrdinal;`
`483`	`488`	`private SqlDataReader _reader;`
`@@ -525,7 +530,8 @@ public long GetChars(long dataIndex, char[] buffer, int bufferIndex, int length)`
`525`	`530`	`}`
`526`	`531`	`else if (dataIndex > _charsRemoved)`
`527`	`532`	`{`
`528`		`- charsToSkip = (int)(dataIndex - _charsRemoved);`
	`533`	`+ //dataIndex is zero-based, but _charsRemoved is one-based, so the difference is the number of chars to skip in the MemoryStream before we start copying data to the buffer`
	`534`	`+ charsToSkip = dataIndex - _charsRemoved;`
`529`	`535`	`}`
`530`	`536`
`531`	`537`	`// If buffer parameter is null, we have to return -1 since there is no way for us to know the`
`@@ -550,7 +556,7 @@ public long GetChars(long dataIndex, char[] buffer, int bufferIndex, int length)`
`550`	`556`	`//_xmlWriter.WriteNode(_xmlReader, true);`
`551`	`557`	`// _xmlWriter.Flush();`
`552`	`558`	`WriteXmlElement();`
`553`		`- // Update memoryStreamRemaining based on the number of chars just written to the MemoryStream`
	`559`	`+ // Update memoryStreamRemaining based on the number of bytes/chars just written to the MemoryStream`
`554`	`560`	`memoryStreamRemaining = _memoryStream.Length - _memoryStream.Position;`
`555`	`561`	`if (charsToSkip > 0)`
`556`	`562`	`{`
`@@ -583,6 +589,7 @@ public long GetChars(long dataIndex, char[] buffer, int bufferIndex, int length)`
`583`	`589`	`cnt = memoryStreamRemaining < length ? memoryStreamRemaining : length;`
`584`	`590`	`for (int i = 0; i < cnt; i++)`
`585`	`591`	`{`
	`592`	`+ // ReadByte moves the Position forward`
`586`	`593`	`buffer[bufferIndex + i] = (char)_memoryStream.ReadByte();`
`587`	`594`	`}`
`588`	`595`	`_charsRemoved += cnt;`
`@@ -598,10 +605,15 @@ private void WriteXmlElement()`
`598`	`605`	`const int WriteNodeBufferSize = 1024;`
`599`	`606`
`600`	`607`	`long memoryStreamPosition = _memoryStream.Position;`
	`608`	`+ // Move the Position to the end of the MemoryStream since we are always appending.`
	`609`	`+ _memoryStream.Seek(0, SeekOrigin.End);`
`601`	`610`
`602`	`611`	`_xmlReader.Read();`
`603`	`612`	`switch (_xmlReader.NodeType)`
`604`	`613`	`{`
	`614`	`+ // Note: Whitespace, CDATA, EntityReference, XmlDeclaration, ProcessingInstruction, DocumentType, and Comment node types`
	`615`	`+ // are not expected in the XML returned from SQL Server as it normalizes them out, but handle them just in case.`
	`616`	`+ // SignificantWhitespace will occur when used with xml:space="preserve"`
`605`	`617`	`case XmlNodeType.Element:`
`606`	`618`	`_xmlWriter.WriteStartElement(_xmlReader.Prefix, _xmlReader.LocalName, _xmlReader.NamespaceURI);`
`607`	`619`	`_xmlWriter.WriteAttributes(_xmlReader, true);`
`@@ -651,6 +663,7 @@ private void WriteXmlElement()`
`651`	`663`	`break;`
`652`	`664`	`}`
`653`	`665`	`_xmlWriter.Flush();`
	`666`	`+ // Reset the Position back to where it was before writing this element so that the caller can continue reading from the expected position.`
`654`	`667`	`_memoryStream.Position = memoryStreamPosition;`
`655`	`668`	`}`
`656`	`669`	`}`