[release/6.0] Enforce HttpClient limits on GetFromJsonAsync (#80552)

* Enforce HttpClient limits on GetFromJsonAsync * Update csproj properties * Fix test csproj
dotnet · Feb 9, 2023 · 0e280c4 · 0e280c4
1 parent 57b695a
commit 0e280c4
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 11 deletions.
diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs
@@ -719,7 +719,7 @@ private async Task<List<byte[]>> ReadRequestHeaderBytesAsync()
             public async Task WriteStringAsync(string s)
             {
                 byte[] bytes = Encoding.ASCII.GetBytes(s);
-                await _stream.WriteAsync(bytes);
+                await _stream.WriteAsync(bytes, 0, bytes.Length);
             }
 
             public async Task SendResponseAsync(string response)
@@ -729,7 +729,7 @@ public async Task SendResponseAsync(string response)
 
             public async Task SendResponseAsync(byte[] response)
             {
-                await _stream.WriteAsync(response);
+                await _stream.WriteAsync(response, 0, response.Length);
             }
 
             public async Task SendResponseAsync(HttpStatusCode statusCode = HttpStatusCode.OK, string additionalHeaders = null, string content = null)

diff --git a/src/libraries/System.Net.Http.Json/src/System.Net.Http.Json.csproj b/src/libraries/System.Net.Http.Json/src/System.Net.Http.Json.csproj
@@ -3,6 +3,8 @@
     <TargetFrameworks>$(NetCoreAppCurrent);net5.0;netstandard2.0;net461</TargetFrameworks>
     <Nullable>enable</Nullable>
     <IsPackable>true</IsPackable>
+    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <ServicingVersion>1</ServicingVersion>
     <PackageDescription>Provides extension methods for System.Net.Http.HttpClient and System.Net.Http.HttpContent that perform automatic serialization and deserialization using System.Text.Json.
 
 Commonly Used Types:

diff --git a/src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/HttpClientJsonExtensions.Get.cs b/src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/HttpClientJsonExtensions.Get.cs
@@ -23,7 +23,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore(taskResponse, type, options, cancellationToken);
         }
 
@@ -35,7 +35,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore(taskResponse, type, options, cancellationToken);
         }
 
@@ -47,7 +47,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore<TValue>(taskResponse, options, cancellationToken);
         }
 
@@ -59,7 +59,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore<TValue>(taskResponse, options, cancellationToken);
         }
 
@@ -70,7 +70,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore(taskResponse, type, context, cancellationToken);
         }
 
@@ -81,7 +81,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore(taskResponse, type, context, cancellationToken);
         }
 
@@ -92,7 +92,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore(taskResponse, jsonTypeInfo, cancellationToken);
         }
 
@@ -103,7 +103,7 @@ public static partial class HttpClientJsonExtensions
                 throw new ArgumentNullException(nameof(client));
             }
 
-            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
+            Task<HttpResponseMessage> taskResponse = client.GetAsync(requestUri, HttpCompletionOption.ResponseContentRead, cancellationToken);
             return GetFromJsonAsyncCore<TValue>(taskResponse, jsonTypeInfo, cancellationToken);
         }
 

diff --git a/src/libraries/System.Net.Http.Json/tests/FunctionalTests/HttpClientJsonExtensionsTests.cs b/src/libraries/System.Net.Http.Json/tests/FunctionalTests/HttpClientJsonExtensionsTests.cs
@@ -257,5 +257,88 @@ await HttpMessageHandlerLoopbackServer.CreateClientAndServerAsync(
                     await server.HandleRequestAsync(content: json, headers: headers);
                 });
         }
+
+        public static IEnumerable<object[]> GetFromJsonAsync_EnforcesMaxResponseContentBufferSize_MemberData() =>
+            from limit in new[] { 2, 100, 100000 }
+            from contentLength in new[] { limit, limit + 1 }
+            from chunked in new[] { true, false }
+            select new object[] { limit, contentLength, chunked };
+
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] // No Socket support
+        [MemberData(nameof(GetFromJsonAsync_EnforcesMaxResponseContentBufferSize_MemberData))]
+        public async Task GetFromJsonAsync_EnforcesMaxResponseContentBufferSize(int limit, int contentLength, bool chunked)
+        {
+            await LoopbackServer.CreateClientAndServerAsync(async uri =>
+            {
+                using var client = new HttpClient { MaxResponseContentBufferSize = limit };
+
+                Func<Task> testMethod = () => client.GetFromJsonAsync<string>(uri);
+
+                if (contentLength > limit)
+                {
+                    Exception ex = await Assert.ThrowsAsync<HttpRequestException>(testMethod);
+                    Assert.Contains(limit.ToString(), ex.Message);
+                }
+                else
+                {
+                    await testMethod();
+                }
+            },
+            async server =>
+            {
+                List<HttpHeaderData> headers = new();
+                string content = $"\"{new string('a', contentLength - 2)}\"";
+
+                if (chunked)
+                {
+                    headers.Add(new HttpHeaderData("Transfer-Encoding", "chunked"));
+                    content = $"{Convert.ToString(contentLength, 16)}\r\n{content}\r\n0\r\n\r\n";
+                }
+
+                await server.HandleRequestAsync(headers: headers, content: content);
+            });
+        }
+
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] // No Socket support
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task GetFromJsonAsync_EnforcesTimeout(bool slowHeaders)
+        {
+            TaskCompletionSource<byte> exceptionThrown = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+            await LoopbackServer.CreateClientAndServerAsync(async uri =>
+            {
+                using var client = new HttpClient { Timeout = TimeSpan.FromMilliseconds(100) };
+
+                Exception ex = await Assert.ThrowsAsync<TaskCanceledException>(() => client.GetFromJsonAsync<string>(uri));
+
+#if NETCORE
+                Assert.Contains("HttpClient.Timeout", ex.Message);
+                Assert.IsType<TimeoutException>(ex.InnerException);
+#endif
+
+                exceptionThrown.SetResult(0);
+            },
+            async server =>
+            {
+                // The client may timeout before even connecting the server
+                await Task.WhenAny(exceptionThrown.Task, Task.Run(async () =>
+                {
+                    try
+                    {
+                        await server.AcceptConnectionAsync(async connection =>
+                        {
+                            if (!slowHeaders)
+                            {
+                                await connection.SendPartialResponseHeadersAsync(headers: new[] { new HttpHeaderData("Content-Length", "42") });
+                            }
+
+                            await exceptionThrown.Task;
+                        });
+                    }
+                    catch { }
+                }));
+            });
+        }
     }
 }
diff --git a/...s/System.Net.Http.Json/tests/FunctionalTests/System.Net.Http.Json.Functional.Tests.csproj b/...s/System.Net.Http.Json/tests/FunctionalTests/System.Net.Http.Json.Functional.Tests.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent);net48</TargetFrameworks>
   </PropertyGroup>
@@ -19,6 +19,7 @@
     <Compile Include="$(CommonTestPath)System\Net\Configuration.Security.cs" Link="Common\System\Net\Configuration.Security.cs" />
     <Compile Include="$(CommonTestPath)System\Net\Http\HttpMessageHandlerLoopbackServer.cs" Link="Common\System\Net\Http\HttpMessageHandlerLoopbackServer.cs" />
     <Compile Include="$(CommonTestPath)System\Net\Http\GenericLoopbackServer.cs" Link="Common\System\Net\Http\GenericLoopbackServer.cs" />
+    <Compile Include="$(CommonTestPath)System\Net\Http\LoopbackServer.cs" Link="Common\System\Net\Http\LoopbackServer.cs" />
     <Compile Include="$(CommonTestPath)System\Security\Cryptography\PlatformSupport.cs" Link="CommonTest\System\Security\Cryptography\PlatformSupport.cs" />
     <Compile Include="$(CommonTestPath)System\Threading\Tasks\TaskTimeoutExtensions.cs" Link="Common\System\Threading\Tasks\TaskTimeoutExtensions.cs" />
   </ItemGroup>