Fix XML parameter encoding for SQL Server

src/EFCore.SqlServer/Storage/Internal/SqlServerStringTypeMapping.cs

@@ -2,7 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Data;
+using System.Data.SqlTypes;
 using System.Text;
+using System.Xml;
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore.Storage.Json;
 
@@ -21,6 +23,15 @@ public class SqlServerStringTypeMapping : StringTypeMapping
 
     private static readonly CaseInsensitiveValueComparer CaseInsensitiveValueComparer = new();
 
+    // Secure by default: DTD processing is prohibited and no external resolver is used so that a malicious
+    // payload is rejected rather than processed.
+    private static readonly XmlReaderSettings XmlFragmentSettings = new()
+    {
+        ConformanceLevel = ConformanceLevel.Fragment,
+        DtdProcessing = DtdProcessing.Prohibit,
+        XmlResolver = null
+    };
+
     private readonly bool _isUtf16;
     private readonly SqlDbType? _sqlDbType;
     private readonly int _maxSpecificSize;
@@ -110,8 +121,9 @@ protected SqlServerStringTypeMapping(RelationalTypeMappingParameters parameters,
             _maxSize = AnsiMax;
         }
 
-        _isUtf16 = parameters.Unicode && parameters.StoreType.StartsWith("n", StringComparison.OrdinalIgnoreCase);
         _sqlDbType = sqlDbType;
+        _isUtf16 = parameters.Unicode
+            && (parameters.StoreType.StartsWith("n", StringComparison.OrdinalIgnoreCase) || _sqlDbType == SqlDbType.Xml);
     }
 
     /// <summary>
@@ -155,6 +167,22 @@ protected override void ConfigureParameter(DbParameter parameter)
             sqlParameter.SqlDbType = _sqlDbType.Value;
         }
 
+        if (_sqlDbType == SqlDbType.Xml)
+        {
+            // SqlClient only sends a parameter as 'xml' (rather than 'nvarchar(max)') when its value is a SqlXml.
+            // Sending it as 'xml' lets SQL Server honor any encoding declared in the XML prolog (e.g. utf-8), which
+            // otherwise fails with "unable to switch the encoding". A fragment-conformant reader is used so that the
+            // content forms that the 'xml' store type accepts - empty strings, text, and multiple top-level nodes -
+            // continue to round-trip.
+            if (value is string xml)
+            {
+                using var reader = XmlReader.Create(new StringReader(xml), XmlFragmentSettings);
+                parameter.Value = new SqlXml(reader);
+            }
+
+            return;
+        }
+
         if ((value == null
                 || value == DBNull.Value)
             || (IsFixedLength

src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs

@@ -120,7 +120,7 @@ private static readonly GuidTypeMapping Uniqueidentifier
         = new("uniqueidentifier");
 
     private static readonly SqlServerStringTypeMapping Xml
-        = new("xml", unicode: true, storeTypePostfix: StoreTypePostfix.None);
+        = new("xml", unicode: true, sqlDbType: SqlDbType.Xml, storeTypePostfix: StoreTypePostfix.None);
 
     private static readonly Dictionary<Type, RelationalTypeMapping> _clrTypeMappings;
 

test/EFCore.SqlServer.FunctionalTests/BuiltInDataTypesSqlServerTest.cs

@@ -4,6 +4,7 @@
 using System.ComponentModel.DataAnnotations;
 using System.ComponentModel.DataAnnotations.Schema;
 using System.Globalization;
+using System.Xml;
 using Microsoft.Data.SqlClient;
 
 // ReSharper disable InconsistentNaming
@@ -3816,6 +3817,103 @@ FROM INFORMATION_SCHEMA.COLUMNS
         return actual;
     }
 
+    // The grinning-face emoji is outside the BMP (a UTF-16 surrogate pair, four UTF-8 bytes) and the euro sign
+    // is a single UTF-16 code unit but three UTF-8 bytes; both are represented differently in UTF-16 than in
+    // UTF-8 and are lost when an xml value is sent to the server as a non-Unicode string, which makes them good
+    // probes for the SqlXml/SqlDbType.Xml parameter path.
+    private const string XmlEmoji = "\U0001F600";
+    private const string XmlEuro = "\u20AC";
+
+    [Theory]
+    [InlineData("<root>" + XmlEmoji + XmlEuro + "</root>", "<root>" + XmlEmoji + XmlEuro + "</root>")]
+    // An explicit non-UTF-16 prolog is accepted because the value is sent as 'xml', not 'nvarchar(max)'.
+    [InlineData("<?xml version=\"1.0\" encoding=\"utf-8\"?><root>" + XmlEmoji + "</root>", "<root>" + XmlEmoji + "</root>")]
+    [InlineData("<?xml version=\"1.0\" encoding=\"utf-16\"?><root>" + XmlEuro + "</root>", "<root>" + XmlEuro + "</root>")]
+    // Content forms that the 'xml' store type accepts beyond a single well-formed document.
+    [InlineData("", "")]
+    [InlineData("text fragment", "text fragment")]
+    [InlineData("<a/><b/>", "<a /><b />")]
+    public async Task Xml_value_round_trips(string value, string expected)
+    {
+        await using var context = CreateContext();
+
+        var document = new XmlTestDocument { Content = value };
+        context.Add(document);
+        await context.SaveChangesAsync();
+
+        var id = document.Id;
+        context.ChangeTracker.Clear();
+
+        // xml columns cannot be compared directly in a WHERE clause, so the row is fetched by its key. Coalescing
+        // the column with the original value sends that value as an 'xml' parameter, exercising the SqlXml
+        // parameter path in a query in addition to the insert above.
+        var roundTripped = await context.Set<XmlTestDocument>()
+            .Where(d => d.Id == id)
+            .Select(d => d.Content ?? value)
+            .SingleAsync();
+        Assert.Equal(expected, roundTripped);
+
+        AssertSql(
+            $"""
+@p0='{expected}' (DbType = Xml)
+
+SET IMPLICIT_TRANSACTIONS OFF;
+SET NOCOUNT ON;
+INSERT INTO [XmlTestDocument] ([Content])
+OUTPUT INSERTED.[Id]
+VALUES (@p0);
+""",
+            //
+            $"""
+@value='{expected}' (DbType = Xml)
+@id='{id}'
+
+SELECT TOP(2) COALESCE([x].[Content], @value)
+FROM [XmlTestDocument] AS [x]
+WHERE [x].[Id] = @id
+""");
+    }
+
+    [Fact]
+    public async Task Xml_value_with_dtd_payload_is_rejected()
+    {
+        await using var context = CreateContext();
+
+        // A "billion laughs" entity-expansion payload: the reader must reject the DTD rather than expand it.
+        const string maliciousXml =
+            "<?xml version=\"1.0\"?>"
+            + "<!DOCTYPE lolz [<!ENTITY lol \"lol\">"
+            + "<!ENTITY lol2 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">"
+            + "<!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">]>"
+            + "<lolz>&lol3;</lolz>";
+
+        context.Add(new XmlTestDocument { Content = maliciousXml });
+
+        var exception = await Assert.ThrowsAnyAsync<Exception>(() => context.SaveChangesAsync());
+        Assert.True(
+            HasXmlException(exception),
+            $"Expected an {nameof(XmlException)} in the exception chain but found: {exception}");
+
+        static bool HasXmlException(Exception exception)
+        {
+            for (var current = exception; current is not null; current = current.InnerException)
+            {
+                if (current is XmlException)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+    }
+
+    private class XmlTestDocument
+    {
+        public int Id { get; set; }
+        public string Content { get; set; }
+    }
+
     private void AssertSql(params string[] expected)
         => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
 
@@ -3897,6 +3995,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                 b.Property(e => e.DecimalAsDec52).HasPrecision(7, 3);
             });
 
+            modelBuilder.Entity<XmlTestDocument>().Property(e => e.Content).HasColumnType("xml");
+
             MakeRequired<MappedDataTypes>(modelBuilder);
             MakeRequired<MappedSquareDataTypes>(modelBuilder);
             MakeRequired<MappedDataTypesWithIdentity>(modelBuilder);

test/EFCore.SqlServer.Tests/Storage/SqlServerTypeMappingTest.cs

@@ -378,6 +378,41 @@ public virtual void Char_Utf8()
         Assert.Equal(DbType.String, parameter.DbType);
     }
 
+    [Theory]
+    [InlineData("<r>a</r>")]
+    [InlineData("<?xml version=\"1.0\" encoding=\"utf-8\"?><r>a</r>")]
+    [InlineData("")]
+    [InlineData("text fragment")]
+    [InlineData("<a/><b/>")]
+    public virtual void Xml_parameter_is_sent_as_SqlXml(string value)
+    {
+        var mapping = GetMapping("xml");
+        Assert.Equal("xml", mapping.StoreType);
+
+        using var command = CreateTestCommand();
+        var parameter = (SqlParameter)mapping.CreateParameter(command, "foo", value);
+
+        Assert.Equal(SqlDbType.Xml, parameter.SqlDbType);
+        var sqlXml = Assert.IsType<System.Data.SqlTypes.SqlXml>(parameter.Value);
+        Assert.False(sqlXml.IsNull);
+    }
+
+    [Fact]
+    public virtual void Xml_null_parameter_is_sent_as_SqlDbType_Xml()
+    {
+        var mapping = GetMapping("xml");
+
+        using var command = CreateTestCommand();
+        var parameter = (SqlParameter)mapping.CreateParameter(command, "foo", null, nullable: true);
+
+        Assert.Equal(SqlDbType.Xml, parameter.SqlDbType);
+        Assert.Equal(DBNull.Value, parameter.Value);
+    }
+
+    [Fact]
+    public virtual void Xml_literal_is_generated_as_unicode()
+        => Test_GenerateSqlLiteral_helper(GetMapping("xml"), "<r>\U0001F62D</r>", "N'<r>\U0001F62D</r>'");
+
     [Fact]
     public virtual void DateOnly_code_literal_generated_correctly()
     {