Throw ArgumentNull instead of NullReference for null requests (#53742)

* Throw ArgumentNull instead of NullReference for null requests * Don't use Send on browser
dotnet · Jun 7, 2021 · d1b5a34 · d1b5a34
1 parent 61587f4
commit d1b5a34
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 32 deletions.
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs
@@ -144,6 +144,11 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request,
 
         protected internal override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
+            if (request == null)
+            {
+                throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest);
+            }
+
             try
             {
                 var requestObject = new JSObject();

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
@@ -552,8 +552,9 @@ private void CheckRequestBeforeSend(HttpRequestMessage request)
         {
             if (request == null)
             {
-                throw new ArgumentNullException(nameof(request));
+                throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest);
             }
+
             CheckDisposed();
             CheckRequestMessage(request);
 

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs
@@ -500,6 +500,11 @@ private HttpMessageHandlerStage SetupHandlerChain()
         protected internal override HttpResponseMessage Send(HttpRequestMessage request,
             CancellationToken cancellationToken)
         {
+            if (request == null)
+            {
+                throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest);
+            }
+
             if (request.Version.Major >= 2)
             {
                 throw new NotSupportedException(SR.Format(SR.net_http_http2_sync_not_supported, GetType()));
@@ -528,6 +533,11 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request,
 
         protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
+            if (request == null)
+            {
+                throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest);
+            }
+
             CheckDisposed();
 
             if (cancellationToken.IsCancellationRequested)

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs
@@ -1101,37 +1101,6 @@ await Assert.ThrowsAnyAsync<Exception>(() =>
             }, UseVersion.ToString()).Dispose();
         }
 
-        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
-        public void SendAsync_NullRequest_ThrowsArgumentNullException()
-        {
-            RemoteExecutor.Invoke(async () =>
-            {
-                var diagnosticListenerObserver = new FakeDiagnosticListenerObserver(null);
-                using (DiagnosticListener.AllListeners.Subscribe(diagnosticListenerObserver))
-                {
-                    diagnosticListenerObserver.Enable();
-
-                    using (MyHandler handler = new MyHandler())
-                    {
-                        // Getting the Task first from the .SendAsync() call also tests
-                        // that the exception comes from the async Task path.
-                        Task t = handler.SendAsync(null);
-                        await Assert.ThrowsAsync<ArgumentNullException>(() => t);
-                    }
-                }
-
-                diagnosticListenerObserver.Disable();
-            }).Dispose();
-        }
-
-        private class MyHandler : HttpClientHandler
-        {
-            internal Task<HttpResponseMessage> SendAsync(HttpRequestMessage request)
-            {
-                return SendAsync(request, CancellationToken.None);
-            }
-        }
-
         private static T GetPropertyValueFromAnonymousTypeInstance<T>(object obj, string propertyName)
         {
             Type t = obj.GetType();

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs
@@ -1428,4 +1428,30 @@ public sealed class HttpClientSendTest_Sync : HttpClientTest.HttpClientSendTest
         public HttpClientSendTest_Sync(ITestOutputHelper output) : base(output) { }
         protected override bool TestAsync => false;
     }
+
+    public sealed class CustomHttpClientTest
+    {
+        private sealed class CustomHttpClient : HttpClientHandler
+        {
+            public Task<HttpResponseMessage> PublicSendAsync(HttpRequestMessage request, CancellationToken cancellationToken = default) =>
+                SendAsync(request, cancellationToken);
+
+            public HttpResponseMessage PublicSend(HttpRequestMessage request, CancellationToken cancellationToken = default) =>
+                Send(request, cancellationToken);
+        }
+
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
+        public void Send_NullRequest_ThrowsException()
+        {
+            using var client = new CustomHttpClient();
+            AssertExtensions.Throws<ArgumentNullException>("request", () => client.PublicSend(null));
+        }
+
+        [Fact]
+        public async Task SendAsync_NullRequest_ThrowsException()
+        {
+            using var client = new CustomHttpClient();
+            await AssertExtensions.ThrowsAsync<ArgumentNullException>("request", () => client.PublicSendAsync(null));
+        }
+    }
 }