Fix XML parameter encoding for SQL Server#38462
Conversation
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…mlReader Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…rateNonNullSqlLiteral test Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…xml literal unit test Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
AndriySvyryd
changed the title
There was a problem hiding this comment.
Pull request overview
This PR improves SQL Server xml handling in EF Core by ensuring XML values are sent as SqlDbType.Xml (using SqlXml) and by hardening the XML parsing path against DTD/XXE-style payloads, with accompanying unit/functional test coverage.
Changes:
- Update SQL Server string type mapping to treat
xmlas Unicode for SQL literal generation and to convert XML string parameters intoSqlXmlusing secureXmlReaderSettings. - Update the SQL Server type mapping source so the
xmlmapping is explicitly configured withSqlDbType.Xml. - Add/extend tests to validate
SqlXmlparameterization, round-tripping behavior, and rejection of DTD payloads.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs | Adds unit tests asserting xml parameters are sent as SqlDbType.Xml and values are SqlXml. |
| test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs | Adds functional round-trip/query coverage for xml values and a negative test for DTD payload rejection; configures an xml-typed entity property in the fixture model. |
| src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs | Configures the xml mapping with sqlDbType: SqlDbType.Xml so parameters can be sent correctly as XML. |
| src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs | Adds secure XML reader settings and converts XML string parameters to SqlXml; adjusts Unicode literal handling for xml. |
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
AndriySvyryd
force-pushed
the
copilot/fix-38429-sqlxml-parameters
branch
from
46e9376 to
3bfc384
Compare
AndriySvyryd
force-pushed
the
copilot/fix-38429-sqlxml-parameters
branch
from
3bfc384 to
9ef8296
Compare
|
|
||
| if (_sqlDbType == SqlDbType.Xml | ||
| && value is string stringValue | ||
| && stringValue.StartsWith(Utf8XmlDeclaration, StringComparison.Ordinal)) |
There was a problem hiding this comment.
This is not a valid way to check a prolog. It perhaps covers a lot of cases, but the version number can now be 1.1 as well as 1.0, and there are many encodings that are not UTF-8 or UTF-16, and the standalone parameter is also optional. And whitespace is optional as well. See https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-prolog-dtd for more info.
If you really want to go down the road of validating the declaration then you will definitely need an XmlReader, but maybe read it manually and dump it as soon as the declaration is confirmed correct. Might be difficult/impossible to rewind it in the case when the encoding needs to change, probably will need a new XmlReader in that case.
There was a problem hiding this comment.
Ok, sounds like just passing the XmlReader in SqlXml would be the more robust solution.
And perhaps you can open an issue for SqlClient to stream it instead of converting to a string.
Fixes #38429
Unicode xml SQL literal:
SqlServerStringTypeMappingnow treats thexmlstore type as Unicode (_isUtf16 = true), soGenerateNonNullSqlLiteralemits anN-prefixed literal. It also setsDbType = Xmlon the parameter and sends the value asSqlXml. Previously a non-Unicode (varchar) literal was generated forxml, silently losing non-ASCII content.