From 9b95e9f49f96b33fa037e8cf08dea292b91a7f51 Mon Sep 17 00:00:00 2001 From: Jan Ruhlaender Date: Sat, 26 Sep 2026 02:43:04 +0200 Subject: [PATCH 1/3] fix: check the status code, honor the encoding and reuse the HttpClient when downloading Behavior change - DownloadToString threw nothing for an error status and returned the content of the error page. TryDownloadToString therefore reported success for a 404. It now throws an HttpRequestException (as DefaultDownloader already did), so TryDownloadToString returns false. Fixes - DownloadToString ignored its encoding parameter. An explicit encoding is now used; without one the charset announced by the server is used (UTF-8 if there is none), which is what happened before. A byte order mark in the content takes precedence in both cases. - HttpChannelDownloader passed UTF-8 as the encoding when none was given. It passes null now, so the server charset keeps being used by default and only an explicit encoding is forced. Performance - DefaultDownloader and DownloaderWithCredentials created a new HttpClient for every download, which exhausts sockets and is slow. DefaultDownloader shares one client, DownloaderWithCredentials creates its client once per instance. - the shared clients recycle their connections every two minutes (SocketsHttpHandler; not available on netstandard2.0), so that a long-lived client notices changed DNS entries - new constructor DefaultDownloader(HttpClient) to configure the client (proxy, timeout, ...); the client is not disposed by the downloader. The parameterless constructor is unchanged. Tests use a fake server (StubHandler) or a local HttpListener with basic authentication; the helpers are shared with DownloadHeaderTest. The IDownloader documentation describes the behavior. --- Core.Test/NetRelated/DefaultDownloaderTest.cs | 110 +++++++++++++++ Core.Test/NetRelated/DownloadHeaderTest.cs | 44 +----- Core.Test/NetRelated/DownloadToStringTest.cs | 130 ++++++++++++++++++ Core.Test/NetRelated/FakeHttpServer.cs | 52 +++++++ Core/Doc/Net/IDownloader.md | 16 +++ Core/Extensions/NetRelated/HttpChannelExt.cs | 26 +++- Core/Net/Impl/DefaultDownloader.cs | 42 ++++-- Core/Net/Impl/DefaultHttpClient.cs | 25 ++++ 8 files changed, 391 insertions(+), 54 deletions(-) create mode 100644 Core.Test/NetRelated/DefaultDownloaderTest.cs create mode 100644 Core.Test/NetRelated/DownloadToStringTest.cs create mode 100644 Core.Test/NetRelated/FakeHttpServer.cs create mode 100644 Core/Net/Impl/DefaultHttpClient.cs diff --git a/Core.Test/NetRelated/DefaultDownloaderTest.cs b/Core.Test/NetRelated/DefaultDownloaderTest.cs new file mode 100644 index 0000000..22a0c7a --- /dev/null +++ b/Core.Test/NetRelated/DefaultDownloaderTest.cs @@ -0,0 +1,110 @@ +using System; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Net.Sockets; +using System.Text; +using System.Threading.Tasks; +using Core.Extensions.NetRelated; +using Core.Net.Impl; +using Xunit; + +namespace Core.Test.NetRelated; + +public class DefaultDownloaderTest +{ + private static HttpResponseMessage Text(string text, HttpStatusCode status = HttpStatusCode.OK) + { + return new HttpResponseMessage(status) { Content = new StringContent(text, Encoding.UTF8, "text/plain") }; + } + + [Fact] + public async Task UsesTheGivenClientAndDoesNotDisposeIt() + { + var handler = new StubHandler(_ => Text("body")); + using var client = new HttpClient(handler); + var downloader = new DefaultDownloader(client); + + Assert.Equal("body", downloader.DownloadToString("https://example.com/a")); + Assert.Equal("body", downloader.DownloadToString("https://example.com/b")); + + Assert.Equal(2, handler.RequestCount); + // a disposed client throws an ObjectDisposedException + Assert.Equal("body", await client.GetStringAsync("https://example.com/c")); + } + + [Fact] + public void FailsForAnErrorStatus() + { + var handler = new StubHandler(_ => Text("error page", HttpStatusCode.NotFound)); + using var client = new HttpClient(handler); + var downloader = new DefaultDownloader(client); + + Assert.False(downloader.TryDownloadToString("https://example.com/missing", out var result, "fallback")); + Assert.Equal("fallback", result); + } + + [Fact] + public void TheClientIsRequired() + { + Assert.Throws(() => new DefaultDownloader(null!)); + } + + [Fact] + public async Task DownloaderWithCredentialsSendsTheCredentialsAndCanBeUsedRepeatedly() + { + var port = GetFreePort(); + var url = $"http://127.0.0.1:{port}/secret"; + var seen = new List(); + + using var listener = new HttpListener { AuthenticationSchemes = AuthenticationSchemes.Basic }; + listener.Prefixes.Add($"http://127.0.0.1:{port}/"); + listener.Start(); + + // the listener answers the first request without credentials with a 401 itself + var server = Task.Run(async () => + { + for (var i = 0; i < 2; i++) + { + var context = await listener.GetContextAsync(); + var identity = (HttpListenerBasicIdentity)context.User!.Identity!; + lock (seen) + seen.Add($"{identity.Name}:{identity.Password}"); + + var body = Encoding.UTF8.GetBytes("secret"); + context.Response.ContentLength64 = body.Length; + await context.Response.OutputStream.WriteAsync(body, 0, body.Length); + context.Response.Close(); + } + }); + + try + { + var downloader = new DownloaderWithCredentials(new NetworkCredential("user", "pass")); + + Assert.Equal("secret", downloader.DownloadToString(url)); + Assert.Equal("secret", downloader.DownloadToString(url)); + + await server.WaitAsync(TimeSpan.FromSeconds(10)); // fails with a TimeoutException if a request never arrives + Assert.Equal(new[] { "user:pass", "user:pass" }, seen); + } + finally + { + listener.Stop(); + } + } + + private static int GetFreePort() + { + var tcp = new TcpListener(IPAddress.Loopback, 0); + tcp.Start(); + try + { + return ((IPEndPoint)tcp.LocalEndpoint).Port; + } + finally + { + tcp.Stop(); + } + } +} diff --git a/Core.Test/NetRelated/DownloadHeaderTest.cs b/Core.Test/NetRelated/DownloadHeaderTest.cs index a801f1d..7f7a5da 100644 --- a/Core.Test/NetRelated/DownloadHeaderTest.cs +++ b/Core.Test/NetRelated/DownloadHeaderTest.cs @@ -42,44 +42,12 @@ private static HttpResponseMessage CreateHeadResponse() return response; } - private sealed class StubHandler : HttpMessageHandler - { - private readonly Func _respond; - - public StubHandler(Func respond) - { - _respond = respond; - } - - public HttpRequestMessage? LastRequest { get; private set; } - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - LastRequest = request; - return Task.FromResult(_respond(request)); - } - } - - private static void WithFakeServer(StubHandler handler, Action action) - { - var original = HttpChannelExt.SharedHttpClient; - HttpChannelExt.SharedHttpClient = new Lazy(() => new HttpClient(handler)); - try - { - action(); - } - finally - { - HttpChannelExt.SharedHttpClient = original; - } - } - [Fact] public void ReadsTheValuesOfTheResponseHeaders() { var handler = new StubHandler(_ => CreateHeadResponse()); - WithFakeServer(handler, () => + SharedHttpClientSwap.Use(handler, () => { var header = new DefaultHttpChannel().DownloadHeader("https://example.com/file"); @@ -96,7 +64,7 @@ public void ReadsTheContentHeaders() { var handler = new StubHandler(_ => CreateHeadResponse()); - WithFakeServer(handler, () => + SharedHttpClientSwap.Use(handler, () => { var header = new DefaultHttpChannel().DownloadHeader("https://example.com/file"); @@ -111,7 +79,7 @@ public void JoinsMultipleValuesOfOneHeader() { var handler = new StubHandler(_ => CreateHeadResponse()); - WithFakeServer(handler, () => + SharedHttpClientSwap.Use(handler, () => { var header = new DefaultHttpChannel().DownloadHeader("https://example.com/file"); @@ -124,7 +92,7 @@ public void NoValueIsTheNameOfAType() { var handler = new StubHandler(_ => CreateHeadResponse()); - WithFakeServer(handler, () => + SharedHttpClientSwap.Use(handler, () => { var header = new DefaultHttpChannel().DownloadHeader("https://example.com/file"); @@ -141,7 +109,7 @@ public void SendsAHeadRequestWithTheAuthorization() var handler = new StubHandler(_ => CreateHeadResponse()); var authorization = new AuthenticationHeaderValue("Bearer", "token"); - WithFakeServer(handler, () => + SharedHttpClientSwap.Use(handler, () => { new DefaultHttpChannel().DownloadHeader("https://example.com/file", authorization); @@ -163,7 +131,7 @@ public void KeepsSeveralSetCookieHeadersApart() return response; }); - WithFakeServer(handler, () => + SharedHttpClientSwap.Use(handler, () => { var header = new DefaultHttpChannel().DownloadHeader("https://example.com/file"); diff --git a/Core.Test/NetRelated/DownloadToStringTest.cs b/Core.Test/NetRelated/DownloadToStringTest.cs new file mode 100644 index 0000000..f8fa698 --- /dev/null +++ b/Core.Test/NetRelated/DownloadToStringTest.cs @@ -0,0 +1,130 @@ +using System.Net; +using System.Net.Http; +using System.Text; +using Core.Extensions.NetRelated; +using Core.Net.Impl; +using Xunit; + +namespace Core.Test.NetRelated; + +/// +/// Tests and the downloader on top of it against a fake server. +/// +[Collection(SharedHttpClientCollection.Name)] +public class DownloadToStringTest +{ + private const string Url = "https://example.com/text"; + private const string Text = "Grüße aus Köln"; + private static readonly Encoding Latin1 = Encoding.GetEncoding("ISO-8859-1"); + + private static HttpResponseMessage Response(byte[] body, string? contentType, HttpStatusCode status = HttpStatusCode.OK) + { + var response = new HttpResponseMessage(status) { Content = new ByteArrayContent(body) }; + if (contentType != null) + response.Content.Headers.TryAddWithoutValidation("Content-Type", contentType); + return response; + } + + private static StubHandler Serve(byte[] body, string? contentType, HttpStatusCode status = HttpStatusCode.OK) + { + return new StubHandler(_ => Response(body, contentType, status)); + } + + [Theory] + [InlineData(HttpStatusCode.NotFound)] + [InlineData(HttpStatusCode.InternalServerError)] + public void ThrowsForAnErrorStatusInsteadOfReturningTheErrorPage(HttpStatusCode status) + { + var handler = Serve(Encoding.UTF8.GetBytes("error page"), "text/html", status); + + SharedHttpClientSwap.Use(handler, () => + { + var exception = Assert.Throws(() => new DefaultHttpChannel().DownloadToString(Url)); + Assert.Contains(((int)status).ToString(), exception.Message); + }); + } + + [Fact] + public void TryDownloadToStringFailsForAnErrorStatus() + { + var handler = Serve(Encoding.UTF8.GetBytes("error page"), "text/html", HttpStatusCode.NotFound); + + SharedHttpClientSwap.Use(handler, () => + { + var downloader = new HttpChannelDownloader(); + + Assert.False(downloader.TryDownloadToString(Url, out var result, "fallback")); + Assert.Equal("fallback", result); + }); + } + + [Fact] + public void UsesTheCharsetOfTheServerWithoutAnExplicitEncoding() + { + var handler = Serve(Latin1.GetBytes(Text), "text/plain; charset=iso-8859-1"); + + SharedHttpClientSwap.Use(handler, () => + Assert.Equal(Text, new DefaultHttpChannel().DownloadToString(Url))); + } + + [Fact] + public void UsesTheExplicitEncoding() + { + // the server does not say which charset it uses, so the default (UTF-8) would garble the text + var handler = Serve(Latin1.GetBytes(Text), "text/plain"); + + SharedHttpClientSwap.Use(handler, () => + Assert.Equal(Text, new DefaultHttpChannel().DownloadToString(Url, Latin1))); + } + + [Fact] + public void TheExplicitEncodingWinsOverTheCharsetOfTheServer() + { + var handler = Serve(Encoding.UTF8.GetBytes(Text), "text/plain; charset=iso-8859-1"); + + SharedHttpClientSwap.Use(handler, () => + Assert.Equal(Text, new DefaultHttpChannel().DownloadToString(Url, Encoding.UTF8))); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void TheByteOrderMarkIsNotPartOfTheText(bool explicitEncoding) + { + var body = new byte[] { 0xEF, 0xBB, 0xBF }; + body = Concat(body, Encoding.UTF8.GetBytes(Text)); + var handler = Serve(body, "text/plain; charset=utf-8"); + + SharedHttpClientSwap.Use(handler, () => + { + var text = new DefaultHttpChannel().DownloadToString(Url, explicitEncoding ? Encoding.UTF8 : null); + Assert.Equal(Text, text); + }); + } + + [Fact] + public void HttpChannelDownloaderUsesTheCharsetOfTheServerByDefault() + { + var handler = Serve(Latin1.GetBytes(Text), "text/plain; charset=iso-8859-1"); + + SharedHttpClientSwap.Use(handler, () => + Assert.Equal(Text, new HttpChannelDownloader().DownloadToString(Url))); + } + + [Fact] + public void HttpChannelDownloaderUsesTheGivenEncoding() + { + var handler = Serve(Latin1.GetBytes(Text), "text/plain"); + + SharedHttpClientSwap.Use(handler, () => + Assert.Equal(Text, new HttpChannelDownloader(encoding: Latin1).DownloadToString(Url))); + } + + private static byte[] Concat(byte[] first, byte[] second) + { + var result = new byte[first.Length + second.Length]; + first.CopyTo(result, 0); + second.CopyTo(result, first.Length); + return result; + } +} diff --git a/Core.Test/NetRelated/FakeHttpServer.cs b/Core.Test/NetRelated/FakeHttpServer.cs new file mode 100644 index 0000000..20ed47f --- /dev/null +++ b/Core.Test/NetRelated/FakeHttpServer.cs @@ -0,0 +1,52 @@ +using System; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using Core.Extensions.NetRelated; + +namespace Core.Test.NetRelated; + +/// +/// Answers every request with the response the given function creates, so no server is needed. +/// +internal sealed class StubHandler : HttpMessageHandler +{ + private readonly Func _respond; + + public StubHandler(Func respond) + { + _respond = respond; + } + + public HttpRequestMessage? LastRequest { get; private set; } + + public int RequestCount { get; private set; } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + LastRequest = request; + RequestCount++; + return Task.FromResult(_respond(request)); + } +} + +internal static class SharedHttpClientSwap +{ + /// + /// Runs the action while uses the handler. + /// The tests that call this must be in the . + /// + public static void Use(HttpMessageHandler handler, Action action) + { + var original = HttpChannelExt.SharedHttpClient; + HttpChannelExt.SharedHttpClient = new Lazy(() => new HttpClient(handler)); + try + { + action(); + } + finally + { + HttpChannelExt.SharedHttpClient = original; + } + } +} diff --git a/Core/Doc/Net/IDownloader.md b/Core/Doc/Net/IDownloader.md index 9084116..45e2acb 100644 --- a/Core/Doc/Net/IDownloader.md +++ b/Core/Doc/Net/IDownloader.md @@ -27,3 +27,19 @@ if (!downloader.TryDownloadToString("https://www.example.com", out var result)) // success: result now contains the downloaded web resource. ``` +## Behavior + +* A download **fails** if the server answers with an error status code (e.g. 404 or 500) or the server can't be reached. + `DownloadToString()` throws an exception, `TryDownloadToString()` returns `false` and `result` is the fallback. + The content of an error page is never returned as a result. +* `DefaultDownloader` uses one `HttpClient` for all downloads of the application. Pass your own client to + configure it (proxy, timeout, ...). It is not disposed by the downloader: + + ```csharp + var downloader = new DefaultDownloader(myHttpClient); + ``` + +* `DownloaderWithCredentials` creates its client with the first download and reuses it afterwards. +* `HttpChannelDownloader` decodes the text with the charset the server announces (UTF-8 if there is none). + Pass an `Encoding` to the constructor to force a specific encoding. A byte order mark in the content takes precedence. + diff --git a/Core/Extensions/NetRelated/HttpChannelExt.cs b/Core/Extensions/NetRelated/HttpChannelExt.cs index 1fbabf6..31ec7da 100644 --- a/Core/Extensions/NetRelated/HttpChannelExt.cs +++ b/Core/Extensions/NetRelated/HttpChannelExt.cs @@ -16,17 +16,31 @@ namespace Core.Extensions.NetRelated; public static class HttpChannelExt { - public static Lazy SharedHttpClient = new Lazy(() => new HttpClient()); + public static Lazy SharedHttpClient = new Lazy(() => DefaultHttpClient.Create()); + /// + /// Downloads the content of the url as text. + /// + /// + /// The encoding of the text. If it is not given, the charset the server announces is used (UTF-8 if there is none). + /// A byte order mark in the content takes precedence over both. + /// + /// The server did not answer with a success status code. public static string DownloadToString(this IHttpChannel channel, string url, Encoding? encoding = default, AuthenticationHeaderValue? authenticationHeaderValue = null) { - encoding = encoding ?? Encoding.UTF8; - var request = channel.CreateRequest(url); + using var request = channel.CreateRequest(url); if (authenticationHeaderValue != null) request.Headers.Authorization = authenticationHeaderValue; - var result = SharedHttpClient.Value.SendAsync(request).Result; - var contentAsString = result.Content.ReadAsStringAsync().Result; - return contentAsString; + + using var result = SharedHttpClient.Value.SendAsync(request).Result; + result.EnsureSuccessStatusCode(); + + if (encoding == null) + return result.Content.ReadAsStringAsync().Result; + + using var stream = result.Content.ReadAsStreamAsync().Result; + using var reader = new StreamReader(stream, encoding, detectEncodingFromByteOrderMarks: true); + return reader.ReadToEnd(); } public static IHttpHeader DownloadHeader(this IHttpChannel channel, string url, AuthenticationHeaderValue? authenticationHeaderValue = null) diff --git a/Core/Net/Impl/DefaultDownloader.cs b/Core/Net/Impl/DefaultDownloader.cs index 0442c28..1d85815 100644 --- a/Core/Net/Impl/DefaultDownloader.cs +++ b/Core/Net/Impl/DefaultDownloader.cs @@ -1,4 +1,5 @@ -using System.Net; +using System; +using System.Net; using System.Net.Http; using System.Text; using Core.Extensions.NetRelated; @@ -7,26 +8,47 @@ namespace Core.Net.Impl; public class DefaultDownloader : IDownloader { + // one client for all downloaders: a client per download exhausts sockets and is slow + private static readonly Lazy SharedClient = new Lazy(() => DefaultHttpClient.Create()); + + private readonly HttpClient? _client; + + /// + /// Downloads with a client that is shared by all downloaders of the application. + /// + public DefaultDownloader() + { + } + + /// + /// Downloads with the given client. The client is not disposed, its lifetime is up to the caller. + /// + public DefaultDownloader(HttpClient httpClient) + { + _client = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); + } + public string DownloadToString(string url) { - using (var client = new HttpClient()) - return client.GetStringAsync(url).Result; + return (_client ?? SharedClient.Value).GetStringAsync(url).Result; } } public class DownloaderWithCredentials : IDownloader { - private readonly ICredentials _credentials; + private readonly Lazy _client; + + /// + /// The client is created with the first download and then reused by this instance. + /// public DownloaderWithCredentials(ICredentials credentials) { - _credentials = credentials; + _client = new Lazy(() => DefaultHttpClient.Create(credentials)); } public string DownloadToString(string url) { - using (var handler = new HttpClientHandler { Credentials = _credentials }) - using (var client = new HttpClient(handler)) - return client.GetStringAsync(url).Result; + return _client.Value.GetStringAsync(url).Result; } } @@ -38,7 +60,7 @@ public HttpChannelDownloader( Encoding? encoding = default) { _httpChannel = httpChannel ?? new DefaultHttpChannel(); - _encoding = encoding ?? Encoding.UTF8; + _encoding = encoding; } public string DownloadToString(string url) @@ -46,6 +68,6 @@ public string DownloadToString(string url) return _httpChannel.DownloadToString(url, _encoding); } - private readonly Encoding _encoding; + private readonly Encoding? _encoding; private readonly IHttpChannel _httpChannel; } \ No newline at end of file diff --git a/Core/Net/Impl/DefaultHttpClient.cs b/Core/Net/Impl/DefaultHttpClient.cs new file mode 100644 index 0000000..2749284 --- /dev/null +++ b/Core/Net/Impl/DefaultHttpClient.cs @@ -0,0 +1,25 @@ +using System; +using System.Net; +using System.Net.Http; + +namespace Core.Net.Impl; + +/// +/// Creates the that is meant to live as long as the application. +/// +internal static class DefaultHttpClient +{ + public static HttpClient Create(ICredentials? credentials = null) + { +#if NETSTANDARD2_0 + return new HttpClient(new HttpClientHandler { Credentials = credentials }); +#else + // connections are recycled regularly, so that a long-lived client notices changed DNS entries + return new HttpClient(new SocketsHttpHandler + { + Credentials = credentials, + PooledConnectionLifetime = TimeSpan.FromMinutes(2) + }); +#endif + } +} From 6e5a82590826defc89cab3910e41ee34581cb399 Mon Sep 17 00:00:00 2001 From: Jan Ruhlaender Date: Sat, 26 Sep 2026 03:09:15 +0200 Subject: [PATCH 2/3] fix: do not share cookies between downloads, throw the real exception Review findings for the shared HttpClient. Cookies (P1) - the shared client of the downloaders used a handler with cookies enabled. A session cookie set for one download was sent with later downloads of other, independent downloaders, so sessions of different users or jobs could get mixed. Before, every download had its own client and was isolated. - clients created by DefaultHttpClient do not store cookies by default. This includes the process-wide HttpChannelExt.SharedHttpClient, which had the same problem before this change (a shared static client with a cookie container). Stateful downloads stay possible with a client the caller passes in (DefaultDownloader(HttpClient)) or by assigning HttpChannelExt.SharedHttpClient. - DefaultDownloader and HttpChannelExt.SharedHttpClient start with the same client (DefaultHttpClient.Shared) and so share one connection pool Exceptions - the synchronous wrappers used .Result, which wraps every failure in an AggregateException, while EnsureSuccessStatusCode threw the HttpRequestException directly. They use GetAwaiter().GetResult() now, so callers always get the HttpRequestException itself. Other - DownloaderWithCredentials requires its credentials (ArgumentNullException) - fix a typo in the IDownloader documentation Tests - DownloaderCookieTest: two independent downloaders, repeated downloads, DownloaderWithCredentials and the default IHttpChannel do not share cookies; a client the caller owns may keep them - LocalHttpServer: Dispose called Stop() and then Close(). The managed HttpListener then removes the prefix a second time, which fails with "Address already in use" while pooled connections of the client are open (about 1 in 20 runs). Close() alone is enough. Ports are also handed out only once per process and a busy port is retried. --- .../10.0.203.aspNetCertificateSentinel | 0 .../.dotnet/10.0.203.dotnetFirstUseSentinel | 0 .dotnet/.dotnet/10.0.203.toolpath.sentinel | 0 .local/share/NuGet/Migrations/1 | 0 Core.Test/NetRelated/DefaultDownloaderTest.cs | 80 +++++------- Core.Test/NetRelated/DownloaderCookieTest.cs | 93 ++++++++++++++ Core.Test/NetRelated/LocalHttpServer.cs | 119 ++++++++++++++++++ Core/Doc/Net/IDownloader.md | 14 ++- Core/Extensions/NetRelated/HttpChannelExt.cs | 10 +- Core/Net/Impl/DefaultDownloader.cs | 16 ++- Core/Net/Impl/DefaultHttpClient.cs | 17 ++- 11 files changed, 285 insertions(+), 64 deletions(-) create mode 100644 .dotnet/.dotnet/10.0.203.aspNetCertificateSentinel create mode 100644 .dotnet/.dotnet/10.0.203.dotnetFirstUseSentinel create mode 100644 .dotnet/.dotnet/10.0.203.toolpath.sentinel create mode 100644 .local/share/NuGet/Migrations/1 create mode 100644 Core.Test/NetRelated/DownloaderCookieTest.cs create mode 100644 Core.Test/NetRelated/LocalHttpServer.cs diff --git a/.dotnet/.dotnet/10.0.203.aspNetCertificateSentinel b/.dotnet/.dotnet/10.0.203.aspNetCertificateSentinel new file mode 100644 index 0000000..e69de29 diff --git a/.dotnet/.dotnet/10.0.203.dotnetFirstUseSentinel b/.dotnet/.dotnet/10.0.203.dotnetFirstUseSentinel new file mode 100644 index 0000000..e69de29 diff --git a/.dotnet/.dotnet/10.0.203.toolpath.sentinel b/.dotnet/.dotnet/10.0.203.toolpath.sentinel new file mode 100644 index 0000000..e69de29 diff --git a/.local/share/NuGet/Migrations/1 b/.local/share/NuGet/Migrations/1 new file mode 100644 index 0000000..e69de29 diff --git a/Core.Test/NetRelated/DefaultDownloaderTest.cs b/Core.Test/NetRelated/DefaultDownloaderTest.cs index 22a0c7a..418adda 100644 --- a/Core.Test/NetRelated/DefaultDownloaderTest.cs +++ b/Core.Test/NetRelated/DefaultDownloaderTest.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Net; using System.Net.Http; -using System.Net.Sockets; using System.Text; using System.Threading.Tasks; using Core.Extensions.NetRelated; @@ -51,60 +50,49 @@ public void TheClientIsRequired() } [Fact] - public async Task DownloaderWithCredentialsSendsTheCredentialsAndCanBeUsedRepeatedly() + public void DownloaderWithCredentialsSendsTheCredentialsAndCanBeUsedRepeatedly() { - var port = GetFreePort(); - var url = $"http://127.0.0.1:{port}/secret"; var seen = new List(); - using var listener = new HttpListener { AuthenticationSchemes = AuthenticationSchemes.Basic }; - listener.Prefixes.Add($"http://127.0.0.1:{port}/"); - listener.Start(); - - // the listener answers the first request without credentials with a 401 itself - var server = Task.Run(async () => - { - for (var i = 0; i < 2; i++) - { - var context = await listener.GetContextAsync(); - var identity = (HttpListenerBasicIdentity)context.User!.Identity!; - lock (seen) - seen.Add($"{identity.Name}:{identity.Password}"); - - var body = Encoding.UTF8.GetBytes("secret"); - context.Response.ContentLength64 = body.Length; - await context.Response.OutputStream.WriteAsync(body, 0, body.Length); - context.Response.Close(); - } - }); - - try + // the listener answers a request without credentials with a 401 itself + using var server = new LocalHttpServer(context => { - var downloader = new DownloaderWithCredentials(new NetworkCredential("user", "pass")); + var identity = (HttpListenerBasicIdentity)context.User!.Identity!; + lock (seen) + seen.Add($"{identity.Name}:{identity.Password}"); + LocalHttpServer.WriteText(context, "secret"); + }, AuthenticationSchemes.Basic); + + var downloader = new DownloaderWithCredentials(new NetworkCredential("user", "pass")); - Assert.Equal("secret", downloader.DownloadToString(url)); - Assert.Equal("secret", downloader.DownloadToString(url)); + Assert.Equal("secret", downloader.DownloadToString(server.Url("/secret"))); + Assert.Equal("secret", downloader.DownloadToString(server.Url("/secret"))); - await server.WaitAsync(TimeSpan.FromSeconds(10)); // fails with a TimeoutException if a request never arrives + lock (seen) Assert.Equal(new[] { "user:pass", "user:pass" }, seen); - } - finally - { - listener.Stop(); - } } - private static int GetFreePort() + [Fact] + public void ThrowsTheHttpRequestExceptionItselfForAnErrorStatus() { - var tcp = new TcpListener(IPAddress.Loopback, 0); - tcp.Start(); - try - { - return ((IPEndPoint)tcp.LocalEndpoint).Port; - } - finally - { - tcp.Stop(); - } + using var client = new HttpClient(new StubHandler(_ => Text("error page", HttpStatusCode.NotFound))); + + // not wrapped in an AggregateException + Assert.Throws(() => new DefaultDownloader(client).DownloadToString("https://example.com/missing")); + } + + [Fact] + public void ThrowsTheHttpRequestExceptionItselfIfTheServerCannotBeReached() + { + using var client = new HttpClient(new StubHandler(_ => throw new HttpRequestException("no route to host"))); + + var exception = Assert.Throws(() => new DefaultDownloader(client).DownloadToString("https://example.com/")); + Assert.Equal("no route to host", exception.Message); + } + + [Fact] + public void TheCredentialsAreRequired() + { + Assert.Throws(() => new DownloaderWithCredentials(null!)); } } diff --git a/Core.Test/NetRelated/DownloaderCookieTest.cs b/Core.Test/NetRelated/DownloaderCookieTest.cs new file mode 100644 index 0000000..3ea32fa --- /dev/null +++ b/Core.Test/NetRelated/DownloaderCookieTest.cs @@ -0,0 +1,93 @@ +using System.Net; +using System.Net.Http; +using Core.Extensions.NetRelated; +using Core.Net.Impl; +using Xunit; + +namespace Core.Test.NetRelated; + +/// +/// The downloaders share their to reuse connections. That must not share state: +/// two independent downloads (users, jobs) must never see each other's cookies. +/// +[Collection(SharedHttpClientCollection.Name)] // uses the process-wide HttpChannelExt.SharedHttpClient +public class DownloaderCookieTest +{ + /// + /// /login sets a session cookie, every other path answers with the cookies the request carried. + /// + private static LocalHttpServer StartServer() + { + return new LocalHttpServer(context => + { + if (context.Request.Url!.AbsolutePath == "/login") + { + context.Response.AppendHeader("Set-Cookie", "session=first-user; Path=/"); + LocalHttpServer.WriteText(context, "logged in"); + } + else + { + LocalHttpServer.WriteText(context, context.Request.Headers["Cookie"] ?? "no cookie"); + } + }); + } + + [Fact] + public void DefaultDownloadersDoNotShareCookies() + { + using var server = StartServer(); + + Assert.Equal("logged in", new DefaultDownloader().DownloadToString(server.Url("/login"))); + + // a different downloader stands for a different user or job + Assert.Equal("no cookie", new DefaultDownloader().DownloadToString(server.Url("/whoami"))); + } + + [Fact] + public void DefaultDownloaderDoesNotKeepCookiesBetweenDownloads() + { + using var server = StartServer(); + var downloader = new DefaultDownloader(); + + downloader.DownloadToString(server.Url("/login")); + + Assert.Equal("no cookie", downloader.DownloadToString(server.Url("/whoami"))); + } + + [Fact] + public void DownloaderWithCredentialsDoesNotKeepCookies() + { + using var server = StartServer(); + var downloader = new DownloaderWithCredentials(new NetworkCredential("user", "pass")); + + downloader.DownloadToString(server.Url("/login")); + + Assert.Equal("no cookie", downloader.DownloadToString(server.Url("/whoami"))); + Assert.Equal("no cookie", new DownloaderWithCredentials(new NetworkCredential("other", "pass")).DownloadToString(server.Url("/whoami"))); + } + + [Fact] + public void AGivenClientMayKeepItsCookies() + { + // stateful downloads are possible on purpose: with a client the caller owns and configures + using var server = StartServer(); + using var client = new HttpClient(new HttpClientHandler { UseCookies = true }); + var downloader = new DefaultDownloader(client); + + downloader.DownloadToString(server.Url("/login")); + + Assert.Equal("session=first-user", downloader.DownloadToString(server.Url("/whoami"))); + } + + [Fact] + public void TheDefaultHttpChannelDoesNotKeepCookies() + { + // HttpChannelExt.SharedHttpClient is process-wide as well + using var server = StartServer(); + var channel = new DefaultHttpChannel(); + + channel.DownloadToString(server.Url("/login")); + + Assert.Equal("no cookie", channel.DownloadToString(server.Url("/whoami"))); + } +} diff --git a/Core.Test/NetRelated/LocalHttpServer.cs b/Core.Test/NetRelated/LocalHttpServer.cs new file mode 100644 index 0000000..b243df7 --- /dev/null +++ b/Core.Test/NetRelated/LocalHttpServer.cs @@ -0,0 +1,119 @@ +using System; +using System.Collections.Generic; +using System.Net; +using System.Net.Sockets; +using System.Threading.Tasks; + +namespace Core.Test.NetRelated; + +/// +/// A small http server on the loopback adapter for tests that need a real connection +/// (cookies, credentials). It handles one request at a time. +/// +internal sealed class LocalHttpServer : IDisposable +{ + // Tests run in parallel. A port is only handed out once per process, otherwise two servers could get the same one. + private static readonly HashSet HandedOutPorts = new(); + + private const int MaxStartAttempts = 10; + + private readonly HttpListener _listener; + private readonly Task _loop; + + public LocalHttpServer(Action handle, AuthenticationSchemes authentication = AuthenticationSchemes.Anonymous) + { + // A free port is only free until somebody else takes it, so starting can fail: try another port then. + for (var attempt = 1; ; attempt++) + { + var port = ReservePort(); + var listener = new HttpListener { AuthenticationSchemes = authentication }; + listener.Prefixes.Add($"http://127.0.0.1:{port}/"); + try + { + listener.Start(); + } + catch (HttpListenerException) when (attempt < MaxStartAttempts) + { + listener.Close(); + continue; + } + + _listener = listener; + BaseUrl = $"http://127.0.0.1:{port}"; + break; + } + + _loop = Task.Run(async () => + { + while (_listener.IsListening) + { + HttpListenerContext context; + try + { + context = await _listener.GetContextAsync(); + } + catch (Exception e) when (e is HttpListenerException or ObjectDisposedException or InvalidOperationException) + { + break; // the listener was stopped + } + + try + { + handle(context); + } + catch (Exception) + { + context.Response.StatusCode = 500; + } + finally + { + context.Response.Close(); + } + } + }); + } + + public string BaseUrl { get; } + + public string Url(string path) => BaseUrl + path; + + public static void WriteText(HttpListenerContext context, string text) + { + var body = System.Text.Encoding.UTF8.GetBytes(text); + context.Response.ContentType = "text/plain; charset=utf-8"; + context.Response.ContentLength64 = body.Length; + context.Response.OutputStream.Write(body, 0, body.Length); + } + + public void Dispose() + { + // Close() stops the listener itself. Calling Stop() first makes Close() remove the prefix a second time, + // which fails with "Address already in use" while connections of a pooled client are still open. + _listener.Close(); + _loop.Wait(TimeSpan.FromSeconds(5)); + } + + private static int ReservePort() + { + lock (HandedOutPorts) + { + while (true) + { + var tcp = new TcpListener(IPAddress.Loopback, 0); + tcp.Start(); + int port; + try + { + port = ((IPEndPoint)tcp.LocalEndpoint).Port; + } + finally + { + tcp.Stop(); + } + + if (HandedOutPorts.Add(port)) + return port; + } + } + } +} diff --git a/Core/Doc/Net/IDownloader.md b/Core/Doc/Net/IDownloader.md index 45e2acb..701e0dd 100644 --- a/Core/Doc/Net/IDownloader.md +++ b/Core/Doc/Net/IDownloader.md @@ -1,6 +1,6 @@ # IDownloader -This interface makes the inplementation to download a file exchangeable. +This interface makes the implementation to download a file exchangeable. ## Interface ```csharp @@ -30,16 +30,20 @@ if (!downloader.TryDownloadToString("https://www.example.com", out var result)) ## Behavior * A download **fails** if the server answers with an error status code (e.g. 404 or 500) or the server can't be reached. - `DownloadToString()` throws an exception, `TryDownloadToString()` returns `false` and `result` is the fallback. + `DownloadToString()` throws the exception itself (an `HttpRequestException`, not wrapped in an `AggregateException`), + `TryDownloadToString()` returns `false` and `result` is the fallback. The content of an error page is never returned as a result. -* `DefaultDownloader` uses one `HttpClient` for all downloads of the application. Pass your own client to - configure it (proxy, timeout, ...). It is not disposed by the downloader: +* `DefaultDownloader` uses one `HttpClient` for all downloads of the application. That client does **not** store cookies, + so downloads of different users or jobs never see each other's session. Pass your own client to configure it + (proxy, timeout, cookies, ...). It is not disposed by the downloader: ```csharp var downloader = new DefaultDownloader(myHttpClient); ``` -* `DownloaderWithCredentials` creates its client with the first download and reuses it afterwards. +* `DownloaderWithCredentials` creates its client with the first download and reuses it afterwards. It does not store cookies either. +* `HttpChannelExt.SharedHttpClient`, which the `IHttpChannel` extension methods use, is the same kind of client and does not store cookies either. + Assign your own client to it if you need a session. * `HttpChannelDownloader` decodes the text with the charset the server announces (UTF-8 if there is none). Pass an `Encoding` to the constructor to force a specific encoding. A byte order mark in the content takes precedence. diff --git a/Core/Extensions/NetRelated/HttpChannelExt.cs b/Core/Extensions/NetRelated/HttpChannelExt.cs index 31ec7da..00e24f2 100644 --- a/Core/Extensions/NetRelated/HttpChannelExt.cs +++ b/Core/Extensions/NetRelated/HttpChannelExt.cs @@ -16,7 +16,7 @@ namespace Core.Extensions.NetRelated; public static class HttpChannelExt { - public static Lazy SharedHttpClient = new Lazy(() => DefaultHttpClient.Create()); + public static Lazy SharedHttpClient = new Lazy(() => DefaultHttpClient.Shared.Value); /// /// Downloads the content of the url as text. @@ -32,13 +32,13 @@ public static string DownloadToString(this IHttpChannel channel, string url, Enc if (authenticationHeaderValue != null) request.Headers.Authorization = authenticationHeaderValue; - using var result = SharedHttpClient.Value.SendAsync(request).Result; + using var result = SharedHttpClient.Value.SendAsync(request).GetAwaiter().GetResult(); result.EnsureSuccessStatusCode(); if (encoding == null) - return result.Content.ReadAsStringAsync().Result; + return result.Content.ReadAsStringAsync().GetAwaiter().GetResult(); - using var stream = result.Content.ReadAsStreamAsync().Result; + using var stream = result.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); using var reader = new StreamReader(stream, encoding, detectEncodingFromByteOrderMarks: true); return reader.ReadToEnd(); } @@ -52,7 +52,7 @@ public static IHttpHeader DownloadHeader(this IHttpChannel channel, string url, request.Method = HttpMethod.Head; - using var result = SharedHttpClient.Value.SendAsync(request).Result; + using var result = SharedHttpClient.Value.SendAsync(request).GetAwaiter().GetResult(); // Header names are case-insensitive. Content-Length, Content-Type, Last-Modified etc. are content headers, // all others response headers, so both collections are needed. A header can have several values. diff --git a/Core/Net/Impl/DefaultDownloader.cs b/Core/Net/Impl/DefaultDownloader.cs index 1d85815..6a58505 100644 --- a/Core/Net/Impl/DefaultDownloader.cs +++ b/Core/Net/Impl/DefaultDownloader.cs @@ -8,13 +8,14 @@ namespace Core.Net.Impl; public class DefaultDownloader : IDownloader { - // one client for all downloaders: a client per download exhausts sockets and is slow - private static readonly Lazy SharedClient = new Lazy(() => DefaultHttpClient.Create()); + // One client for all downloaders: a client per download exhausts sockets and is slow. + // It does not keep cookies: every download stands for its own user or job and must not see the session of another. + private static Lazy SharedClient => DefaultHttpClient.Shared; private readonly HttpClient? _client; /// - /// Downloads with a client that is shared by all downloaders of the application. + /// Downloads with a client that is shared by all downloaders of the application. It does not store cookies. /// public DefaultDownloader() { @@ -22,6 +23,7 @@ public DefaultDownloader() /// /// Downloads with the given client. The client is not disposed, its lifetime is up to the caller. + /// Use this for downloads that need cookies: the client decides whether they are stored and sent. /// public DefaultDownloader(HttpClient httpClient) { @@ -30,7 +32,7 @@ public DefaultDownloader(HttpClient httpClient) public string DownloadToString(string url) { - return (_client ?? SharedClient.Value).GetStringAsync(url).Result; + return (_client ?? SharedClient.Value).GetStringAsync(url).GetAwaiter().GetResult(); } } @@ -39,16 +41,18 @@ public class DownloaderWithCredentials : IDownloader private readonly Lazy _client; /// - /// The client is created with the first download and then reused by this instance. + /// The client is created with the first download and then reused by this instance. It does not store cookies. /// public DownloaderWithCredentials(ICredentials credentials) { + if (credentials == null) throw new ArgumentNullException(nameof(credentials)); + _client = new Lazy(() => DefaultHttpClient.Create(credentials)); } public string DownloadToString(string url) { - return _client.Value.GetStringAsync(url).Result; + return _client.Value.GetStringAsync(url).GetAwaiter().GetResult(); } } diff --git a/Core/Net/Impl/DefaultHttpClient.cs b/Core/Net/Impl/DefaultHttpClient.cs index 2749284..8e47a6b 100644 --- a/Core/Net/Impl/DefaultHttpClient.cs +++ b/Core/Net/Impl/DefaultHttpClient.cs @@ -9,15 +9,28 @@ namespace Core.Net.Impl; /// internal static class DefaultHttpClient { - public static HttpClient Create(ICredentials? credentials = null) + /// + /// The client all downloaders and HttpChannelExt.SharedHttpClient start with, so they share one connection pool. + /// It does not store cookies. + /// + public static readonly Lazy Shared = new Lazy(() => Create()); + + /// The credentials for servers that ask for authentication. + /// + /// Whether the client stores the cookies of the servers and sends them with later requests. + /// A client that is shared or reused must not do that unless all its users are meant to share one session, + /// so it is off by default. + /// + public static HttpClient Create(ICredentials? credentials = null, bool useCookies = false) { #if NETSTANDARD2_0 - return new HttpClient(new HttpClientHandler { Credentials = credentials }); + return new HttpClient(new HttpClientHandler { Credentials = credentials, UseCookies = useCookies }); #else // connections are recycled regularly, so that a long-lived client notices changed DNS entries return new HttpClient(new SocketsHttpHandler { Credentials = credentials, + UseCookies = useCookies, PooledConnectionLifetime = TimeSpan.FromMinutes(2) }); #endif From 16e2cbc4a269b5cc17fda3151291ece58c9f601d Mon Sep 17 00:00:00 2001 From: Jan Ruhlaender Date: Sat, 26 Sep 2026 03:15:08 +0200 Subject: [PATCH 3/3] fix: correct the byte order mark contract, dispose the header request Review findings. - the documentation claimed that a byte order mark takes precedence in every case. That is only true with an explicit encoding (StreamReader detects the mark). Without one, HttpContent.ReadAsStringAsync decides: it uses the charset the server announces and ignores a mark of another encoding (UTF-8 content with a mark and "charset=iso-8859-1" is decoded as Latin-1). This path is unchanged from before, so the docs are corrected and not the code - DownloadHeader disposes its request like DownloadToString does - IDownloader docs: the fallback of TryDownloadToString is "" and not default; note that netstandard2.0 cannot recycle connections, so hosts there that follow DNS changes should pass their own client Tests: an explicit encoding loses against a byte order mark; a client passed to DefaultDownloader keeps its timeout; an already disposed client fails when it is used. --- Core.Test/NetRelated/DefaultDownloaderTest.cs | 30 +++++++++++++++++++ Core.Test/NetRelated/DownloadToStringTest.cs | 11 +++++++ Core/Doc/Net/IDownloader.md | 7 +++-- Core/Extensions/NetRelated/HttpChannelExt.cs | 8 +++-- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/Core.Test/NetRelated/DefaultDownloaderTest.cs b/Core.Test/NetRelated/DefaultDownloaderTest.cs index 418adda..fcdd1a4 100644 --- a/Core.Test/NetRelated/DefaultDownloaderTest.cs +++ b/Core.Test/NetRelated/DefaultDownloaderTest.cs @@ -95,4 +95,34 @@ public void TheCredentialsAreRequired() { Assert.Throws(() => new DownloaderWithCredentials(null!)); } + + [Fact] + public void HonorsTheTimeoutOfTheGivenClient() + { + using var client = new HttpClient(new HangingHandler()) { Timeout = TimeSpan.FromMilliseconds(100) }; + + Assert.ThrowsAny(() => new DefaultDownloader(client).DownloadToString("https://example.com/slow")); + } + + [Fact] + public void AnAlreadyDisposedClientFailsWhenItIsUsed() + { + var client = new HttpClient(new StubHandler(_ => Text("body"))); + client.Dispose(); + + // the downloader does not own the client, so it does not check it: using it is the caller's job + Assert.Throws(() => new DefaultDownloader(client).DownloadToString("https://example.com/")); + } + + /// + /// Never answers, so only the timeout of the client ends the request. + /// + private sealed class HangingHandler : HttpMessageHandler + { + protected override async Task SendAsync(HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) + { + await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, cancellationToken); + return new HttpResponseMessage(); + } + } } diff --git a/Core.Test/NetRelated/DownloadToStringTest.cs b/Core.Test/NetRelated/DownloadToStringTest.cs index f8fa698..bc405c8 100644 --- a/Core.Test/NetRelated/DownloadToStringTest.cs +++ b/Core.Test/NetRelated/DownloadToStringTest.cs @@ -102,6 +102,17 @@ public void TheByteOrderMarkIsNotPartOfTheText(bool explicitEncoding) }); } + [Fact] + public void AByteOrderMarkWinsOverTheExplicitEncoding() + { + // the caller says Latin-1, but the content is UTF-8 with a byte order mark: the mark tells the truth + var body = Concat(new byte[] { 0xEF, 0xBB, 0xBF }, Encoding.UTF8.GetBytes(Text)); + var handler = Serve(body, "text/plain"); + + SharedHttpClientSwap.Use(handler, () => + Assert.Equal(Text, new DefaultHttpChannel().DownloadToString(Url, Latin1))); + } + [Fact] public void HttpChannelDownloaderUsesTheCharsetOfTheServerByDefault() { diff --git a/Core/Doc/Net/IDownloader.md b/Core/Doc/Net/IDownloader.md index 701e0dd..da4e2a9 100644 --- a/Core/Doc/Net/IDownloader.md +++ b/Core/Doc/Net/IDownloader.md @@ -9,7 +9,7 @@ public interface IDownloader string DownloadToString(string url); // via extension methods - bool TryDownloadToString(string url, out string result, string fallback = default) + bool TryDownloadToString(string url, out string result, string fallback = "") } ``` @@ -45,5 +45,8 @@ if (!downloader.TryDownloadToString("https://www.example.com", out var result)) * `HttpChannelExt.SharedHttpClient`, which the `IHttpChannel` extension methods use, is the same kind of client and does not store cookies either. Assign your own client to it if you need a session. * `HttpChannelDownloader` decodes the text with the charset the server announces (UTF-8 if there is none). - Pass an `Encoding` to the constructor to force a specific encoding. A byte order mark in the content takes precedence. + Pass an `Encoding` to the constructor to force a specific encoding. A byte order mark in the content still wins over + that encoding. If the server announces a charset, a byte order mark is not looked at (as in `HttpContent.ReadAsStringAsync`). +* On `netstandard2.0` (e.g. .NET Framework) the shared clients cannot recycle their connections: a long-lived client keeps using + the address it resolved first until the connection breaks. Hosts that need to follow DNS changes there should pass their own client. diff --git a/Core/Extensions/NetRelated/HttpChannelExt.cs b/Core/Extensions/NetRelated/HttpChannelExt.cs index 00e24f2..49a3995 100644 --- a/Core/Extensions/NetRelated/HttpChannelExt.cs +++ b/Core/Extensions/NetRelated/HttpChannelExt.cs @@ -22,8 +22,10 @@ public static class HttpChannelExt /// Downloads the content of the url as text. /// /// - /// The encoding of the text. If it is not given, the charset the server announces is used (UTF-8 if there is none). - /// A byte order mark in the content takes precedence over both. + /// The encoding of the text. If it is given, it is used, unless the content starts with a byte order mark: that decides then. + /// If it is not given, the charset the server announces is used. Without one the text is UTF-8, or the encoding + /// a byte order mark stands for. A byte order mark is not looked at when the server announces a charset + /// (as does). /// /// The server did not answer with a success status code. public static string DownloadToString(this IHttpChannel channel, string url, Encoding? encoding = default, AuthenticationHeaderValue? authenticationHeaderValue = null) @@ -45,7 +47,7 @@ public static string DownloadToString(this IHttpChannel channel, string url, Enc public static IHttpHeader DownloadHeader(this IHttpChannel channel, string url, AuthenticationHeaderValue? authenticationHeaderValue = null) { - var request = channel.CreateRequest(url); + using var request = channel.CreateRequest(url); if (authenticationHeaderValue != null) request.Headers.Authorization = authenticationHeaderValue;