Obsolete XmlSecureResolver

docs/project/list-of-diagnostics.md

@@ -101,6 +101,7 @@ The PR that reveals the implementation of the `<IncludeInternalObsoleteAttribute
 |  __`SYSLIB0044`__ | AssemblyName.CodeBase and AssemblyName.EscapedCodeBase are obsolete. Using them for loading an assembly is not supported. |
 |  __`SYSLIB0045`__ | Cryptographic factory methods accepting an algorithm name are obsolete. Use the parameterless Create factory method on the algorithm type instead. |
 |  __`SYSLIB0046`__ | ControlledExecution.Run method may corrupt the process and should not be used in production code. |
+|  __`SYSLIB0047`__ | XmlSecureResolver is obsolete. Use XmlResolver.ThrowingResolver instead when attempting to forbid XML external entity resolution. |
 
 ## Analyzer Warnings
 

src/libraries/Common/src/System/Obsoletions.cs

@@ -150,5 +150,8 @@ internal static class Obsoletions
 
         internal const string ControlledExecutionRunMessage = "ControlledExecution.Run method may corrupt the process and should not be used in production code.";
         internal const string ControlledExecutionRunDiagId = "SYSLIB0046";
+
+        internal const string XmlSecureResolverMessage = "XmlSecureResolver is obsolete. Use XmlResolver.ThrowingResolver instead when attempting to forbid XML external entity resolution.";
+        internal const string XmlSecureResolverDiagId = "SYSLIB0047";
     }
 }

src/libraries/System.Private.Xml/src/Resources/Strings.resx

@@ -514,7 +514,7 @@
     <value>Object type is not supported.</value>
   </data>
   <data name="Xml_NullResolver" xml:space="preserve">
-    <value>Resolving of external URIs was prohibited.</value>
+    <value>Resolving of external URIs was prohibited. Attempted access to: {0}</value>
   </data>
   <data name="Xml_RelativeUriNotSupported" xml:space="preserve">
     <value>Relative URIs are not supported.</value>

src/libraries/System.Private.Xml/src/System.Private.Xml.csproj

@@ -127,13 +127,11 @@
     <Compile Include="System\Xml\XmlNameTable.cs" />
     <Compile Include="System\Xml\XmlNodeOrder.cs" />
     <Compile Include="System\Xml\XmlNodeType.cs" />
-    <Compile Include="System\Xml\XmlNullResolver.cs" />
     <Compile Include="System\Xml\XmlQualifiedName.cs" />
     <Compile Include="System\Xml\XmlReservedNs.cs" />
     <Compile Include="System\Xml\XmlResolver.cs" />
-    <Compile Include="System\Xml\XmlResolverAsync.cs" />
+    <Compile Include="System\Xml\XmlResolver.ThrowingResolver.cs" />
     <Compile Include="System\Xml\XmlSecureResolver.cs" />
-    <Compile Include="System\Xml\XmlSecureResolverAsync.cs" />
     <Compile Include="System\Xml\XmlUrlResolver.cs" />
     <Compile Include="System\Xml\XmlUrlResolverAsync.cs" />
     <Compile Include="System\Xml\Core\CharEntityEncoderFallback.cs" />
@@ -748,6 +746,7 @@
     <Compile Include="$(CommonPath)System\CSharpHelpers.cs" />
     <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.cs" Link="Common\System\Text\ValueStringBuilder.cs" />
     <Compile Include="$(CommonPath)System\Text\ValueStringBuilder.AppendSpanFormattable.cs" Link="Common\System\Text\ValueStringBuilder.AppendSpanFormattable.cs" />
+    <Compile Include="$(CommonPath)System\Obsoletions.cs" Link="Common\System\Obsoletions.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'windows'">

src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.ThrowingResolver.cs

@@ -0,0 +1,49 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Net;
+using System.Threading.Tasks;
+
+namespace System.Xml
+{
+    public abstract partial class XmlResolver
+    {
+        /// <summary>
+        /// Gets an XML resolver which forbids entity resolution.
+        /// </summary>
+        /// <value>An XML resolver which forbids entity resolution.</value>
+        /// <remarks>
+        /// Calling <see cref="GetEntity"/> or <see cref="GetEntityAsync"/> on the
+        /// <see cref="XmlResolver"/> instance returned by this property is forbidden
+        /// and will result in <see cref="XmlException"/> being thrown.
+        ///
+        /// Use <see cref="ThrowingResolver"/> when external entity resolution must be
+        /// prohibited, even when DTD processing is otherwise enabled.
+        /// </remarks>
+        public static XmlResolver ThrowingResolver => XmlThrowingResolver.s_singleton;
+
+        // An XmlResolver that forbids all external entity resolution.
+        private sealed class XmlThrowingResolver : XmlResolver
+        {
+            internal static readonly XmlThrowingResolver s_singleton = new();
+
+            // Private constructor ensures existing only one instance of XmlThrowingResolver
+            private XmlThrowingResolver() { }
+
+            public override ICredentials Credentials
+            {
+                set { /* Do nothing */ }
+            }
+
+            public override object GetEntity(Uri absoluteUri, string? role, Type? ofObjectToReturn)
+            {
+                throw new XmlException(SR.Format(SR.Xml_NullResolver, absoluteUri));
+            }
+
+            public override Task<object> GetEntityAsync(Uri absoluteUri, string? role, Type? ofObjectToReturn)
+            {
+                throw new XmlException(SR.Format(SR.Xml_NullResolver, absoluteUri));
+            }
+        }
+    }
+}

src/libraries/System.Private.Xml/src/System/Xml/XmlResolver.cs

@@ -28,7 +28,12 @@ public abstract partial class XmlResolver
                                           string? role,
                                           Type? ofObjectToReturn);
 
-
+        public virtual Task<object> GetEntityAsync(Uri absoluteUri,
+                                                   string? role,
+                                                   Type? ofObjectToReturn)
+        {
+            throw new NotImplementedException();
+        }
 
         /// <devdoc>
         ///    <para>[To be supplied.]</para>

src/libraries/System.Private.Xml/src/System/Xml/XmlSecureResolver.cs

@@ -3,32 +3,31 @@
 
 using System.Net;
 using System.Security;
-using System.Runtime.Versioning;
+using System.Threading.Tasks;
 
 namespace System.Xml
 {
-    public partial class XmlSecureResolver : XmlResolver
+    [Obsolete(Obsoletions.XmlSecureResolverMessage, DiagnosticId = Obsoletions.XmlSecureResolverDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
+    public class XmlSecureResolver : XmlResolver
     {
-        private readonly XmlResolver _resolver;
-
         public XmlSecureResolver(XmlResolver resolver, string? securityUrl)
         {
-            _resolver = resolver;
+            // no-op
         }
 
         public override ICredentials Credentials
         {
-            set { _resolver.Credentials = value; }
+            set { /* no-op */ }
         }
 
-        public override object? GetEntity(Uri absoluteUri, string? role, Type? ofObjectToReturn)
-        {
-            return _resolver.GetEntity(absoluteUri, role, ofObjectToReturn);
-        }
+        // Forward to ThrowingResolver to get its exception message
+        public override object? GetEntity(Uri absoluteUri, string? role, Type? ofObjectToReturn) => XmlResolver.ThrowingResolver.GetEntity(absoluteUri, role, ofObjectToReturn);
 
-        public override Uri ResolveUri(Uri? baseUri, string? relativeUri)
-        {
-            return _resolver.ResolveUri(baseUri, relativeUri);
-        }
+        // Forward to ThrowingResolver to get its exception message
+        public override Task<object> GetEntityAsync(Uri absoluteUri, string? role, Type? ofObjectToReturn) => XmlResolver.ThrowingResolver.GetEntityAsync(absoluteUri, role, ofObjectToReturn);
+
+        // An earlier implementation of this type overrode this method, so we'll continue to do so
+        // to preserve binary compatibility.
+        public override Uri ResolveUri(Uri? baseUri, string? relativeUri) => base.ResolveUri(baseUri, relativeUri);
     }
 }

src/libraries/System.Private.Xml/src/System/Xml/Xsl/XmlILCommand.cs

@@ -83,7 +83,7 @@ private void Execute(object defaultDocument, XmlResolver? dataSources, XsltArgum
             Debug.Assert(results != null);
 
             // Ensure that dataSources is always non-null
-            dataSources ??= XmlNullResolver.Singleton;
+            dataSources ??= XmlResolver.ThrowingResolver;
 
             _delExec(new XmlQueryRuntime(_staticData, defaultDocument, dataSources, argumentList, results));
         }

src/libraries/System.Private.Xml/src/System/Xml/Xsl/Xslt/Compiler.cs

@@ -70,12 +70,12 @@ public Compiler(XsltSettings settings, bool debug, string? scriptAssemblyPath)
             Scripts = new Scripts(this);
         }
 
-        public CompilerErrorCollection Compile(object stylesheet, XmlResolver? xmlResolver, out QilExpression qil)
+        public CompilerErrorCollection Compile(object stylesheet, XmlResolver? xmlResolver, XmlResolver? origResolver, out QilExpression qil)
         {
             Debug.Assert(stylesheet != null);
             Debug.Assert(Root == null, "Compiler cannot be reused");
 
-            new XsltLoader().Load(this, stylesheet, xmlResolver);
+            new XsltLoader().Load(this, stylesheet, xmlResolver, origResolver);
             qil = QilGenerator.CompileStylesheet(this);
             SortErrors();
             return CompilerErrorColl;

src/libraries/System.Private.Xml/src/System/Xml/Xsl/Xslt/XsltLoader.cs

@@ -38,11 +38,11 @@ internal sealed class XsltLoader : IErrorHelper
         public static int V2Opt = 4;
         public static int V2Req = 8;
 
-        public void Load(Compiler compiler, object stylesheet, XmlResolver? xmlResolver)
+        public void Load(Compiler compiler, object stylesheet, XmlResolver? xmlResolver, XmlResolver? origResolver)
         {
             Debug.Assert(compiler != null);
             _compiler = compiler;
-            _xmlResolver = xmlResolver ?? XmlNullResolver.Singleton;
+            _xmlResolver = xmlResolver ?? XmlResolver.ThrowingResolver;
 
             XmlReader? reader = stylesheet as XmlReader;
             if (reader != null)
@@ -57,10 +57,21 @@ public void Load(Compiler compiler, object stylesheet, XmlResolver? xmlResolver)
                 string? uri = stylesheet as string;
                 if (uri != null)
                 {
-                    // If xmlResolver == null, then the original uri will be resolved using XmlUrlResolver
-                    XmlResolver origResolver = xmlResolver!;
-                    if (xmlResolver == null || xmlResolver == XmlNullResolver.Singleton)
-                        origResolver = new XmlUrlResolver();
+                    // If the stylesheet has been provided as a string (URI, really), then we'll bounce
+                    // through an XmlResolver to look up its contents. There's a complication here since
+                    // the default resolver provided by our caller is likely a throwing resolver, and
+                    // a throwing resolver would fail even when attempting to read this URL. We need
+                    // to at minimum allow this URL to be read, because the user after all did explicitly
+                    // ask us to do so.
+                    //
+                    // In this case, we'll rely on the 'origResolver' argument, which is the XmlResolver
+                    // which was provided *to our caller* before any default substitution took place.
+                    // If an explicit resolver was specified, we'll honor it. Otherwise we'll substitute
+                    // an XmlUrlResolver for this one read operation. The stored resolver (which is used
+                    // for reads beyond the initial stylesheet read) will use a throwing resolver as its
+                    // default, as shown at the very top of this method.
+
+                    origResolver ??= new XmlUrlResolver();
                     Uri resolvedUri = origResolver.ResolveUri(null, uri);
                     if (resolvedUri == null)
                     {

src/libraries/System.Private.Xml/src/System/Xml/Xsl/XsltOld/Processor.cs

@@ -345,7 +345,7 @@ public Processor(
             _builder = null;
             _actionStack = new HWStack(StackIncrement);
             _output = _rootAction.Output;
-            _resolver = resolver ?? XmlNullResolver.Singleton;
+            _resolver = resolver ?? XmlResolver.ThrowingResolver;
             _args = args ?? new XsltArgumentList();
             _debugger = debugger;
             if (_debugger != null)

src/libraries/System.Private.Xml/src/System/Xml/Xslt/XslCompiledTransform.cs

@@ -82,53 +82,56 @@ private void Reset()
         public void Load(XmlReader stylesheet)
         {
             Reset();
-            LoadInternal(stylesheet, XsltSettings.Default, CreateDefaultResolver());
+            LoadInternal(stylesheet, XsltSettings.Default, CreateDefaultResolver(), originalStylesheetResolver: null);
         }
 
         // SxS: This method does not take any resource name and does not expose any resources to the caller.
         // It's OK to suppress the SxS warning.
         public void Load(XmlReader stylesheet, XsltSettings? settings, XmlResolver? stylesheetResolver)
         {
             Reset();
-            LoadInternal(stylesheet, settings, stylesheetResolver);
+            LoadInternal(stylesheet, settings, stylesheetResolver, stylesheetResolver);
         }
 
         // SxS: This method does not take any resource name and does not expose any resources to the caller.
         // It's OK to suppress the SxS warning.
         public void Load(IXPathNavigable stylesheet)
         {
             Reset();
-            LoadInternal(stylesheet, XsltSettings.Default, CreateDefaultResolver());
+            LoadInternal(stylesheet, XsltSettings.Default, CreateDefaultResolver(), originalStylesheetResolver: null);
         }
 
         // SxS: This method does not take any resource name and does not expose any resources to the caller.
         // It's OK to suppress the SxS warning.
         public void Load(IXPathNavigable stylesheet, XsltSettings? settings, XmlResolver? stylesheetResolver)
         {
             Reset();
-            LoadInternal(stylesheet, settings, stylesheetResolver);
+            LoadInternal(stylesheet, settings, stylesheetResolver, stylesheetResolver);
         }
 
         public void Load(string stylesheetUri)
         {
             Reset();
             ArgumentNullException.ThrowIfNull(stylesheetUri);
-            LoadInternal(stylesheetUri, XsltSettings.Default, CreateDefaultResolver());
+            LoadInternal(stylesheetUri, XsltSettings.Default, CreateDefaultResolver(), originalStylesheetResolver: null);
         }
 
         public void Load(string stylesheetUri, XsltSettings? settings, XmlResolver? stylesheetResolver)
         {
             Reset();
             ArgumentNullException.ThrowIfNull(stylesheetUri);
-            LoadInternal(stylesheetUri, settings, stylesheetResolver);
+            LoadInternal(stylesheetUri, settings, stylesheetResolver, stylesheetResolver);
         }
 
-        private void LoadInternal(object stylesheet, XsltSettings? settings, XmlResolver? stylesheetResolver)
+        // The 'originalStylesheetResolver' argument should be the original XmlResolver
+        // that was passed to the caller (or null), *before* any default substitutions
+        // were made by the caller.
+        private void LoadInternal(object stylesheet, XsltSettings? settings, XmlResolver? stylesheetResolver, XmlResolver? originalStylesheetResolver)
         {
             ArgumentNullException.ThrowIfNull(stylesheet);
 
             settings ??= XsltSettings.Default;
-            CompileXsltToQil(stylesheet, settings, stylesheetResolver);
+            CompileXsltToQil(stylesheet, settings, stylesheetResolver, originalStylesheetResolver);
             CompilerError? error = GetFirstError();
             if (error != null)
             {
@@ -142,9 +145,9 @@ private void LoadInternal(object stylesheet, XsltSettings? settings, XmlResolver
 
         [MemberNotNull(nameof(_compilerErrorColl))]
         [MemberNotNull(nameof(_qil))]
-        private void CompileXsltToQil(object stylesheet, XsltSettings settings, XmlResolver? stylesheetResolver)
+        private void CompileXsltToQil(object stylesheet, XsltSettings settings, XmlResolver? stylesheetResolver, XmlResolver? originalStylesheetResolver)
         {
-            _compilerErrorColl = new Compiler(settings, _enableDebug, null).Compile(stylesheet, stylesheetResolver, out _qil);
+            _compilerErrorColl = new Compiler(settings, _enableDebug, null).Compile(stylesheet, stylesheetResolver, originalStylesheetResolver, out _qil);
         }
 
         /// <summary>
@@ -405,7 +408,7 @@ private static XmlResolver CreateDefaultResolver()
                 return new XmlUrlResolver();
             }
 
-            return XmlNullResolver.Singleton;
+            return XmlResolver.ThrowingResolver;
         }
     }
 }

src/libraries/System.Private.Xml/src/System/Xml/Xslt/XslTransform.cs

@@ -246,7 +246,7 @@ private void Compile(XPathNavigator stylesheet, XmlResolver? resolver)
 
             Compiler compiler = new Compiler();
             NavigatorInput input = new NavigatorInput(stylesheet);
-            compiler.Compile(input, resolver ?? XmlNullResolver.Singleton);
+            compiler.Compile(input, resolver ?? XmlResolver.ThrowingResolver);
 
             Debug.Assert(compiler.CompiledStylesheet != null);
             Debug.Assert(compiler.QueryStore != null);
@@ -264,7 +264,7 @@ private static XmlResolver CreateDefaultResolver()
             }
             else
             {
-                return XmlNullResolver.Singleton;
+                return XmlResolver.ThrowingResolver;
             }
         }
     }