dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.75k forks source link

XSLTs are not possible in NativeAOT #84389

Open eerhardt opened 1 year ago

eerhardt commented 1 year ago

Today to do XSLTs we have XslCompiledTransform. But this class will only ever use IL Ref.Emit to generate the transform. This is not possible when publishing for Native AOT - Ref.Emit won't work.

We also have XslTransform which sounds like it would be a good substitute when IsDynamicCodeSupported=false. However, we have an analyzer that tells devs that this class is not secure and shouldn't be used - XslCompiledTransform should be used instead. https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca5374

So developers who need to use XSLTs in NativeAOT apps don't have a viable option in .NET.

We should solve this somehow - either by making XslCompiledTransform work in Native AOT, or by making XslTransform secure. (Probably the first approach would be best.)

cc @agocke, @MichalStrehovsky, @jkotas

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-xml See info in area-owners.md if you want to be subscribed.

Issue Details
Today to do XSLTs we have `XslCompiledTransform`. But this class will only ever use IL Ref.Emit to generate the transform. This is not possible when publishing for Native AOT - Ref.Emit won't work. We also have `XslTransform` which sounds like it would be a good substitute when `IsDynamicCodeSupported=false`. However, we have an analyzer that tells devs that this class is not secure and shouldn't be used - `XslCompiledTransform` should be used instead. https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca5374 So developers who need to use XSLTs in NativeAOT apps don't have a viable option in .NET. We should solve this somehow - either by making `XslCompiledTransform` work in Native AOT, or by making `XslTransform` secure. (Probably the first approach would be best.) cc @agocke, @MichalStrehovsky, @jkotas
Author: eerhardt
Assignees: -
Labels: `area-System.Xml`
Milestone: -
jkotas commented 1 year ago

XSLT support is de-facto archived. We are way behind the current standard (we implement XSLT 1.0 published in 1999, current version is XSLT 3.0 published in 2017). We have no plans to invest into it to make it current.

What are the modern scenarios where XSLT matters?

eerhardt commented 1 year ago

What are the modern scenarios where XSLT matters?

The place I hit it was in another "de-facto archived" library System.Security.Cryptography.Xml in trying to fix https://github.com/dotnet/runtime/issues/73432:

https://github.com/dotnet/runtime/blob/18e2c5fd9e2239a8b06fe49dbb6492d40f5e5e19/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CryptoHelpers.cs#L19

The reason I am even caring about System.Security.Cryptography.Xml is because ASP.NET's DataProtection uses EncryptedXml. See https://github.com/dotnet/aspnetcore/issues/47410 for more information on why this is needed. We are trying to enable developers to use DataProtection in NativeAOT apps.

To work around this issue, I have 2 ideas for solutions:

  1. Annotate System.Security.Cryptography.Xml as RequiresDynamicCode because of this one possibility that there is a reference to "http://www.w3.org/TR/1999/REC-xslt-19991116" in the XML file.
  2. Throw exceptions early when an xslt element is discovered, for example:
            if (!RuntimeFeature.IsDynamicCodeSupported)
            {
                // XSLTs are only supported when dynamic code is supported. See https://github.com/dotnet/runtime/issues/84389
                throw new NotSupportedException("Xslt Transforms are not supported when DynamicCode is not supported.");
            }
            return new XmlDsigXsltTransform();

    This would mean that dotnet run and F5 would still throw exceptions when the dev had PublishAot=true in their .csproj. But it goes against our use strategy of static analysis to tell devs when their app may not work.

I started down path (2), but since all of System.Security.Cryptography.Xml is going to need to be RequiresUnreferencedCode, I figure adding RequiresDynamicCode isn't going to hurt anything. Doing both is probably the best of both worlds. Having the annotation so devs get warnings. And then throwing early when dynamic code isn’t supported allows for the dead code to be trimmed.

Wraith2 commented 1 year ago

Is there a clear way to make XsltCompiledTransform work in AOT?