[release/7.0] Fix XmlSqlBinaryReader and introduce a corpus of SqlXml tests (#81878) #82063
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #81878 to
release/7.0
. See #82062 for the corresponding backport torelease/6.0
.Customer Impact
As reported in #74852, a regression was introduced in .NET 6 with #43379 that prevents
SqlXml.CreateReader()
from processing SQL Binary XML content (see MS-BINXML).The regression is caused by seeking beyond the end of the current token during token validation. This is due to a simple mistake of using
_end
instead of_pos
when_end
represents the end of the entire buffer and_pos
represents the position 1 byte beyond the end of the current token.We currently have 2 customers reporting that their migrations to .NET 6 are blocked by this regression. While we have not yet obtained their validation that the .NET 8 builds work for them, our collective confidence in the fix is high. We will get their validation as soon as possible, but we wanted to get this fix queued up for the March servicing releases rather than waiting for that validation.
Testing
While the root cause and fix for this regression were well understood since @WaynePaiMS submitted #74852, this issue surfaced a substantial test hole. We did not have any tests validating that SQL Binary XML content could be processed. Such tests existed in .NET Framework, but they had not been ported to .NET Core+.
The fix includes reintroduction of the same collection of tests that were used in .NET Framework, including regression tests that had been added over time. The test cases include collections of SQL Binary and Text XML files. A new unit test processes the test files in both forms, validating that the processed XML matches the results of processing the text form directly through
XmlTextReader
(withoutSqlXml.CreateReader()
). The tests were reviewed for thoroughness and named to match the scenarios they cover.The details of those test cases can be seen here:
https://github.com/dotnet/runtime-assets/tree/main/src/System.Data.Common.TestData
Regression
Yes. The regression categorically prevents SQL XML Binary data from being processed, and this is currently blocking customers from migrating from .NET Core 3.1 and .NET 5.0 to .NET 6 or .NET 7.
Risk
Low. SQL Binary XML processing is categorically broken in .NET 6 and .NET 7. The root cause and fix are understood, and the fix introduces a body of tests to ensure
SqlXml.CreateReader()
produces the same result for SQL Binary XML files as it does for Text XML files, and those tests also ensure that it's processing of Text XML files matches the results of usingXmlTextReader
directly.