Remove System.Net.Requests dependency from System.Private.Xml (dotnet/corefx#41111)

* Remove System.Net.Requests dependency from System.Private.Xml

Anything that uses System.Xml ends up implicitly referencing this .dll, which in a trimmed default MVC app is 97K.  The only thing it's used for is as part of XmlResolver to download the specified url.  We can instead remove the usage of WebRequest.Create and replace it with usage of HttpClient, which is already brought in because System.Net.Requests uses it to implement HttpWebRequest.

* Address PR feedback

And other minor cleanup.  Also fix uap build.

Commit migrated from https://github.com/dotnet/corefx/commit/f873a33056ab318c02d295f816065a3d2ad3736b

diff --git a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
index 2cf48e6..b7f582b 100644 (file)
--- a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
+++ b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
@@ -580,7 +580,6 @@
     <Reference Include="System.Linq.Expressions" />
     <Reference Include="System.Memory" />
     <Reference Include="System.Net.Http" />
-    <Reference Include="System.Net.Requests" />
     <Reference Include="System.Net.Primitives" />
     <Reference Include="System.ObjectModel" />
     <Reference Include="System.Reflection.Primitives" />

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManager.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManager.cs
index 38be47f..8ee06b1 100644 (file)
--- a/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManager.cs
+++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManager.cs
@@ -2,25 +2,15 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
+using System.IO;
+using System.Net;
+
 namespace System.Xml
 {
-    using System;
-    using System.IO;
-    using System.Security;
-    using System.Collections;
-    using System.Net;
-    using System.Net.Cache;
-    using System.Runtime.CompilerServices;
-    using System.Runtime.Versioning;
-    using System.Net.Http;
-
-    //
-    // XmlDownloadManager
-    //
     internal partial class XmlDownloadManager
     {
-        internal Stream GetStream(Uri uri, ICredentials credentials, IWebProxy proxy,
-            RequestCachePolicy cachePolicy)
+        internal Stream GetStream(Uri uri, ICredentials credentials, IWebProxy proxy)
         {
             if (uri.Scheme == "file")
             {
@@ -28,46 +18,10 @@ namespace System.Xml
             }
             else
             {
-                return GetNonFileStream(uri, credentials, proxy, cachePolicy);
-            }
-        }
-
-        private Stream GetNonFileStream(Uri uri, ICredentials credentials, IWebProxy proxy,
-            RequestCachePolicy cachePolicy)
-        {
-            WebRequest req = CreateWebRequestOrThrowIfRemoved(uri, credentials, proxy, cachePolicy);
-
-            using (WebResponse resp = req.GetResponse())
-            using (Stream respStream = resp.GetResponseStream())
-            {
-                var result = new MemoryStream();
-                respStream.CopyTo(result);
-                result.Position = 0;
-                return result;
-            }
-        }
-
-        // This code is statically reachable from any place that uses XmlReaderSettings (i.e. every app that
-        // does something XML related is going to have this in their transitive call graph). People rarely need
-        // this functionality though.
-        private static WebRequest CreateWebRequestOrThrowIfRemoved(Uri uri, ICredentials credentials, IWebProxy proxy,
-            RequestCachePolicy cachePolicy)
-        {
-            WebRequest req = WebRequest.Create(uri);
-            if (credentials != null)
-            {
-                req.Credentials = credentials;
+                // This code should be changed if HttpClient ever gets real synchronous methods.  For now,
+                // we just use the asynchronous methods and block waiting for them to complete.
+                return GetNonFileStreamAsync(uri, credentials, proxy).GetAwaiter().GetResult();
             }
-            if (proxy != null)
-            {
-                req.Proxy = proxy;
-            }
-            if (cachePolicy != null)
-            {
-                req.CachePolicy = cachePolicy;
-            }
-
-            return req;
         }
     }
 }

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManagerAsync.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManagerAsync.cs
index 88d917c..4c51ff1 100644 (file)
--- a/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManagerAsync.cs
+++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlDownloadManagerAsync.cs
@@ -2,47 +2,50 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
+using System.IO;
+using System.Net;
+using System.Net.Http;
+using System.Threading.Tasks;
+
 namespace System.Xml
 {
-    using System;
-    using System.IO;
-    using System.Security;
-    using System.Collections;
-    using System.Net;
-    using System.Net.Cache;
-    using System.Runtime.Versioning;
-    using System.Threading.Tasks;
-    using System.Net.Http;
-    //
-    // XmlDownloadManager
-    //
     internal partial class XmlDownloadManager
     {
-        internal Task<Stream> GetStreamAsync(Uri uri, ICredentials credentials, IWebProxy proxy,
-            RequestCachePolicy cachePolicy)
+        internal Task<Stream> GetStreamAsync(Uri uri, ICredentials credentials, IWebProxy proxy)
         {
             if (uri.Scheme == "file")
             {
-                return Task.Run<Stream>(() => { return new FileStream(uri.LocalPath, FileMode.Open, FileAccess.Read, FileShare.Read, 1, true); });
+                Uri fileUri = uri;
+                return Task.Run<Stream>(() => new FileStream(fileUri.LocalPath, FileMode.Open, FileAccess.Read, FileShare.Read, 1, useAsync: true));
             }
             else
             {
-                return GetNonFileStreamAsync(uri, credentials, proxy, cachePolicy);
+                return GetNonFileStreamAsync(uri, credentials, proxy);
             }
         }
 
-        private async Task<Stream> GetNonFileStreamAsync(Uri uri, ICredentials credentials, IWebProxy proxy,
-            RequestCachePolicy cachePolicy)
+        private async Task<Stream> GetNonFileStreamAsync(Uri uri, ICredentials credentials, IWebProxy proxy)
         {
-            WebRequest req = CreateWebRequestOrThrowIfRemoved(uri, credentials, proxy, cachePolicy);
-
-            using (WebResponse resp = await req.GetResponseAsync().ConfigureAwait(false))
-            using (Stream respStream = resp.GetResponseStream())
+            var handler = new HttpClientHandler();
+            using (var client = new HttpClient(handler))
             {
-                var result = new MemoryStream();
-                await respStream.CopyToAsync(result).ConfigureAwait(false);
-                result.Position = 0;
-                return result;
+                if (credentials != null)
+                {
+                    handler.Credentials = credentials;
+                }
+                if (proxy != null)
+                {
+                    handler.Proxy = proxy;
+                }
+
+                using (Stream respStream = await client.GetStreamAsync(uri).ConfigureAwait(false))
+                {
+                    var result = new MemoryStream();
+                    respStream.CopyTo(result);
+                    result.Position = 0;
+                    return result;
+                }
             }
         }
     }

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolver.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolver.cs
index ddcf2d6..d2537bd 100644 (file)
--- a/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolver.cs
+++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolver.cs
@@ -12,30 +12,16 @@ namespace System.Xml
     // Resolves external XML resources named by a Uniform Resource Identifier (URI).
     public partial class XmlUrlResolver : XmlResolver
     {
-        private static object s_DownloadManager;
+        private static XmlDownloadManager s_downloadManager;
         private ICredentials _credentials;
         private IWebProxy _proxy;
-        private RequestCachePolicy _cachePolicy;
 
-        private static XmlDownloadManager DownloadManager
-        {
-            get
-            {
-                if (s_DownloadManager == null)
-                {
-                    object dm = new XmlDownloadManager();
-                    Interlocked.CompareExchange<object>(ref s_DownloadManager, dm, null);
-                }
-                return (XmlDownloadManager)s_DownloadManager;
-            }
-        }
+        private static XmlDownloadManager DownloadManager =>
+            s_downloadManager ??
+            Interlocked.CompareExchange(ref s_downloadManager, new XmlDownloadManager(), null) ??
+            s_downloadManager;
 
-        // Construction
-
-        // Creates a new instance of the XmlUrlResolver class.
-        public XmlUrlResolver()
-        {
-        }
+        public XmlUrlResolver() { }
 
         public override ICredentials Credentials
         {
@@ -49,27 +35,21 @@ namespace System.Xml
 
         public RequestCachePolicy CachePolicy
         {
-            set { _cachePolicy = value; }
+            set { } // nop, as caching isn't implemented
         }
 
-        // Resource resolution
-
         // Maps a URI to an Object containing the actual resource.
         public override object GetEntity(Uri absoluteUri, string role, Type ofObjectToReturn)
         {
-            if (ofObjectToReturn == null || ofObjectToReturn == typeof(System.IO.Stream) || ofObjectToReturn == typeof(object))
-            {
-                return DownloadManager.GetStream(absoluteUri, _credentials, _proxy, _cachePolicy);
-            }
-            else
+            if (ofObjectToReturn is null || ofObjectToReturn == typeof(System.IO.Stream) || ofObjectToReturn == typeof(object))
             {
-                throw new XmlException(SR.Xml_UnsupportedClass, string.Empty);
+                return DownloadManager.GetStream(absoluteUri, _credentials, _proxy);
             }
-        }
 
-        public override Uri ResolveUri(Uri baseUri, string relativeUri)
-        {
-            return base.ResolveUri(baseUri, relativeUri);
+            throw new XmlException(SR.Xml_UnsupportedClass, string.Empty);
         }
+
+        public override Uri ResolveUri(Uri baseUri, string relativeUri) =>
+            base.ResolveUri(baseUri, relativeUri);
     }
 }

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolverAsync.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolverAsync.cs
index 0fbcabf..53e3c97 100644 (file)
--- a/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolverAsync.cs
+++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlUrlResolverAsync.cs
@@ -14,12 +14,10 @@ namespace System.Xml
         {
             if (ofObjectToReturn == null || ofObjectToReturn == typeof(System.IO.Stream) || ofObjectToReturn == typeof(object))
             {
-                return await DownloadManager.GetStreamAsync(absoluteUri, _credentials, _proxy, _cachePolicy).ConfigureAwait(false);
-            }
-            else
-            {
-                throw new XmlException(SR.Xml_UnsupportedClass, string.Empty);
+                return await DownloadManager.GetStreamAsync(absoluteUri, _credentials, _proxy).ConfigureAwait(false);
             }
+
+            throw new XmlException(SR.Xml_UnsupportedClass, string.Empty);
         }
     }
 }

diff --git a/src/libraries/System.Private.Xml/tests/XmlReader/Tests/AsyncReaderLateInitTests.cs b/src/libraries/System.Private.Xml/tests/XmlReader/Tests/AsyncReaderLateInitTests.cs
index e8bce59..97036bd 100644 (file)
--- a/src/libraries/System.Private.Xml/tests/XmlReader/Tests/AsyncReaderLateInitTests.cs
+++ b/src/libraries/System.Private.Xml/tests/XmlReader/Tests/AsyncReaderLateInitTests.cs
@@ -74,7 +74,7 @@ namespace System.Xml.Tests
         {
             using (XmlReader reader = XmlReader.Create("http://test.test/test.html", new XmlReaderSettings() { Async = true }))
             {
-                Assert.Throws<System.Net.WebException>(() => reader.ReadAsync().GetAwaiter().GetResult());
+                Assert.Throws<System.Net.Http.HttpRequestException>(() => reader.ReadAsync().GetAwaiter().GetResult());
             }
         }
 
@@ -83,14 +83,14 @@ namespace System.Xml.Tests
         {
             using (XmlReader reader = XmlReader.Create("http://test.test/test.html", new XmlReaderSettings() { Async = true }))
             {
-                Assert.Throws<System.Net.WebException>(() => reader.Read());
+                Assert.Throws<System.Net.Http.HttpRequestException>(() => reader.Read());
             }
         }
 
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsSubsystemForLinux))] // [ActiveIssue(11057)]
         public static void InitializationWithUriOnNonAsyncReaderTrows()
         {
-            Assert.Throws<System.Net.WebException>(() => XmlReader.Create("http://test.test/test.html", new XmlReaderSettings() { Async = false }));
+            Assert.Throws<System.Net.Http.HttpRequestException>(() => XmlReader.Create("http://test.test/test.html", new XmlReaderSettings() { Async = false }));
         }
 
         [Fact]

diff --git a/src/libraries/System.Private.Xml/tests/XmlReader/XmlResolver/XmlSystemPathResolverTests.cs b/src/libraries/System.Private.Xml/tests/XmlReader/XmlResolver/XmlSystemPathResolverTests.cs
index 8c711ba..811571a 100644 (file)
--- a/src/libraries/System.Private.Xml/tests/XmlReader/XmlResolver/XmlSystemPathResolverTests.cs
+++ b/src/libraries/System.Private.Xml/tests/XmlReader/XmlResolver/XmlSystemPathResolverTests.cs
@@ -90,10 +90,13 @@ namespace System.Xml.Tests
             AssertInvalidPath("??");
         }
 
-        [Fact]
-        public static void TestResolveInvalidPath()
+        [Theory]
+        [InlineData("http://notfound.invalid.corp.microsoft.com")]
+        [InlineData("ftp://host.invalid")]
+        [InlineData("notsupported://host.invalid")]
+        public static void TestResolveInvalidPath(string invalidUri)
         {
-            Assert.Throws<System.Net.WebException>(() => XmlReader.Create("ftp://host.invalid"));
+            Assert.ThrowsAny<Exception>(() => XmlReader.Create(invalidUri));
         }
 
         [Fact]