Skip to content

Commit efbe56e

Browse files
mjrousosmjrousos
authored
Rework configuring timeout on remote app API
Replace NetworkTimeout with the ability to just provide an HttpClient as per API review. Also fixed an issue with paths on configured base URLs getting dropped. Fixes #133 Fixes #138
1 parent 71bf759 commit efbe56e

15 files changed

+64
-51
lines changed

‎Microsoft.AspNetCore.SystemWebAdapters.sln

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
1616
.editorconfig = .editorconfig
1717
Directory.Build.props = Directory.Build.props
1818
Directory.Build.targets = Directory.Build.targets
19+
global.json = global.json
1920
README.md = README.md
2021
EndProjectSection
2122
EndProject

‎samples/MvcApp/MvcApp.csproj

+2-2
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@
184184
<HintPath>..\..\packages\System.Security.Principal.Windows.5.0.0\lib\net461\System.Security.Principal.Windows.dll</HintPath>
185185
</Reference>
186186
<Reference Include="System.ServiceProcess" />
187-
<Reference Include="System.Text.Encodings.Web, Version=4.0.3.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
188-
<HintPath>..\..\packages\System.Text.Encodings.Web.4.5.0\lib\netstandard2.0\System.Text.Encodings.Web.dll</HintPath>
187+
<Reference Include="System.Text.Encodings.Web, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
188+
<HintPath>..\..\packages\System.Text.Encodings.Web.6.0.0\lib\netstandard2.0\System.Text.Encodings.Web.dll</HintPath>
189189
</Reference>
190190
<Reference Include="System.Threading.Tasks.Extensions, Version=4.2.0.1, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
191191
<HintPath>..\..\packages\System.Threading.Tasks.Extensions.4.5.4\lib\net461\System.Threading.Tasks.Extensions.dll</HintPath>

‎samples/MvcApp/Web.config

+8
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@
3535
</system.web>
3636
<runtime>
3737
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
38+
<dependentAssembly>
39+
<assemblyIdentity name="System.Text.Encodings.Web" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
40+
<bindingRedirect oldVersion="0.0.0.0-6.0.0.0" newVersion="6.0.0.0"/>
41+
</dependentAssembly>
42+
<dependentAssembly>
43+
<assemblyIdentity name="System.Buffers" publicKeyToken="CC7B13FFCD2DDD51" culture="neutral"/>
44+
<bindingRedirect oldVersion="0.0.0.0-4.0.3.0" newVersion="4.0.3.0"/>
45+
</dependentAssembly>
3846
<dependentAssembly>
3947
<assemblyIdentity name="System.ComponentModel.Annotations" publicKeyToken="B03F5F7F11D50A3A" culture="neutral" />
4048
<bindingRedirect oldVersion="0.0.0.0-4.2.1.0" newVersion="4.2.1.0" />

‎samples/MvcApp/packages.config

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
<package id="System.Security.Cryptography.Xml" version="6.0.0" targetFramework="net472" />
5757
<package id="System.Security.Permissions" version="4.5.0" targetFramework="net472" />
5858
<package id="System.Security.Principal.Windows" version="5.0.0" targetFramework="net472" />
59-
<package id="System.Text.Encodings.Web" version="4.5.0" targetFramework="net472" />
59+
<package id="System.Text.Encodings.Web" version="6.0.0" targetFramework="net472" />
6060
<package id="System.Threading.Tasks.Extensions" version="4.5.4" targetFramework="net472" />
6161
<package id="System.ValueTuple" version="4.5.0" targetFramework="net472" />
6262
<package id="WebGrease" version="1.6.0" targetFramework="net472" />

‎samples/RemoteAuth/Forms/FormsAuth/FormsAuth.csproj

-6
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,6 @@
7272
<Private>True</Private>
7373
<HintPath>..\..\..\..\packages\Microsoft.Web.Infrastructure.1.0.0.0\lib\net40\Microsoft.Web.Infrastructure.dll</HintPath>
7474
</Reference>
75-
<Reference Include="AspNet.ScriptManager.bootstrap">
76-
<HintPath>..\..\..\..\packages\AspNet.ScriptManager.bootstrap.3.4.1\lib\net45\AspNet.ScriptManager.bootstrap.dll</HintPath>
77-
</Reference>
78-
<Reference Include="AspNet.ScriptManager.jQuery">
79-
<HintPath>..\..\..\..\packages\AspNet.ScriptManager.jQuery.3.4.1\lib\net45\AspNet.ScriptManager.jQuery.dll</HintPath>
80-
</Reference>
8175
<Reference Include="Microsoft.ScriptManager.MSAjax">
8276
<HintPath>..\..\..\..\packages\Microsoft.AspNet.ScriptManager.MSAjax.5.0.0\lib\net45\Microsoft.ScriptManager.MSAjax.dll</HintPath>
8377
</Reference>

‎samples/RemoteAuth/Forms/FormsAuth/packages.config

-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<packages>
33
<package id="Antlr" version="3.5.0.2" targetFramework="net472" />
4-
<package id="AspNet.ScriptManager.bootstrap" version="3.4.1" targetFramework="net472" />
5-
<package id="AspNet.ScriptManager.jQuery" version="3.4.1" targetFramework="net472" />
64
<package id="bootstrap" version="3.4.1" targetFramework="net472" />
75
<package id="jQuery" version="3.4.1" targetFramework="net472" />
86
<package id="Microsoft.AspNet.FriendlyUrls" version="1.0.2" targetFramework="net472" />

‎src/Microsoft.AspNetCore.SystemWebAdapters.SessionState/RemoteSession/RemoteAppSessionStateClientOptions.cs

-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.ComponentModel.DataAnnotations;
5-
using System;
65

76
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession;
87

@@ -16,11 +15,4 @@ public class RemoteAppSessionStateClientOptions
1615
/// </summary>
1716
[Required]
1817
public string CookieName { get; set; } = SessionConstants.DefaultCookieName;
19-
20-
/// <summary>
21-
/// The maximum time loading session state from the remote app
22-
/// or committing changes to it can take before timing out.
23-
/// </summary>
24-
[Required]
25-
public TimeSpan NetworkTimeout { get; set; } = TimeSpan.FromMinutes(1);
2618
}

‎src/Microsoft.AspNetCore.SystemWebAdapters.SessionState/RemoteSession/RemoteAppSessionStateExtensions.cs

+5-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Net.Http;
66
using Microsoft.AspNetCore.SystemWebAdapters;
7+
using Microsoft.AspNetCore.SystemWebAdapters.SessionState;
78
using Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession;
89

910
namespace Microsoft.Extensions.DependencyInjection;
@@ -17,9 +18,12 @@ public static ISystemWebAdapterRemoteClientAppBuilder AddSession(this ISystemWeb
1718
throw new ArgumentNullException(nameof(builder));
1819
}
1920

20-
builder.Services.AddHttpClient<ISessionManager, RemoteAppSessionStateManager>()
21+
builder.Services.AddHttpClient(SessionConstants.SessionClientName)
2122
// Disable cookies in the HTTP client because the service will manage the cookie header directly
2223
.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { UseCookies = false });
24+
25+
builder.Services.AddTransient<ISessionManager, RemoteAppSessionStateManager>();
26+
2327
builder.Services.AddOptions<RemoteAppSessionStateClientOptions>()
2428
.Configure(configure ?? (_ => { }))
2529
.ValidateDataAnnotations();

‎src/Microsoft.AspNetCore.SystemWebAdapters.SessionState/RemoteSession/RemoteAppSessionStateManager.cs

+27-17
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,42 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession;
1717

1818
internal partial class RemoteAppSessionStateManager : ISessionManager
1919
{
20-
private readonly HttpClient _client;
20+
private readonly IHttpClientFactory _httpClientFactory;
2121
private readonly ISessionSerializer _serializer;
2222
private readonly ILogger<RemoteAppSessionStateManager> _logger;
2323
private readonly RemoteAppSessionStateClientOptions _options;
24+
private readonly IOptions<RemoteAppOptions> _remoteAppOptions;
25+
private HttpClient? _client;
26+
27+
private HttpClient Client
28+
{
29+
get
30+
{
31+
if (_client is null)
32+
{
33+
// Use the HttpClient supplied in options if one is present in options;
34+
// otherwise, generate a client with an IHttpClientFactory from DI
35+
_client = _remoteAppOptions.Value.BackchannelHttpClient ?? _httpClientFactory.CreateClient(SessionConstants.SessionClientName);
36+
_client.BaseAddress = new Uri($"{_remoteAppOptions.Value.RemoteAppUrl.ToString().TrimEnd('/')}{_options.SessionEndpointPath}");
37+
_client.DefaultRequestHeaders.Add(_remoteAppOptions.Value.ApiKeyHeader, _remoteAppOptions.Value.ApiKey);
38+
}
39+
40+
return _client;
41+
}
42+
}
2443

2544
public RemoteAppSessionStateManager(
26-
HttpClient client,
45+
IHttpClientFactory httpClientFactory,
2746
ISessionSerializer serializer,
2847
IOptions<RemoteAppSessionStateClientOptions> sessionOptions,
2948
IOptions<RemoteAppOptions> remoteAppOptions,
3049
ILogger<RemoteAppSessionStateManager> logger)
3150
{
32-
_client = client ?? throw new ArgumentNullException(nameof(client));
33-
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
51+
_remoteAppOptions = remoteAppOptions ?? throw new ArgumentNullException(nameof(remoteAppOptions));
3452
_options = sessionOptions?.Value ?? throw new ArgumentNullException(nameof(sessionOptions));
53+
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
54+
_httpClientFactory = httpClientFactory ?? throw new ArgumentNullException(nameof(httpClientFactory));
3555
_serializer = serializer ?? throw new ArgumentNullException(nameof(serializer));
36-
37-
var remoteOptions = remoteAppOptions?.Value ?? throw new ArgumentNullException(nameof(remoteAppOptions));
38-
_client.BaseAddress = new Uri(remoteOptions.RemoteAppUrl, _options.SessionEndpointPath);
39-
_client.DefaultRequestHeaders.Add(remoteOptions.ApiKeyHeader, remoteOptions.ApiKey);
4056
}
4157

4258
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "Loaded {Count} items from remote session state for session {SessionId}")]
@@ -53,9 +69,6 @@ public RemoteAppSessionStateManager(
5369

5470
public async Task<ISessionState> CreateAsync(HttpContextCore context, SessionAttribute metadata)
5571
{
56-
using var timeout = new CancellationTokenSource(_options.NetworkTimeout);
57-
using var cts = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token, context.RequestAborted, context.RequestAborted);
58-
5972
// If an existing remote session ID is present in the request, use its session ID.
6073
// Otherwise, leave session ID null for now; it will be provided by the remote service
6174
// when session data is loaded.
@@ -64,7 +77,7 @@ public async Task<ISessionState> CreateAsync(HttpContextCore context, SessionAtt
6477
try
6578
{
6679
// Get or create session data
67-
var response = await GetSessionDataAsync(sessionId, metadata.IsReadOnly, context, cts.Token);
80+
var response = await GetSessionDataAsync(sessionId, metadata.IsReadOnly, context, context.RequestAborted);
6881

6982
LogSessionLoad(response.Count, response.SessionID);
7083

@@ -87,7 +100,7 @@ private async Task<ISessionState> GetSessionDataAsync(string? sessionId, bool re
87100
AddSessionCookieToHeader(req, sessionId);
88101
AddReadOnlyHeader(req, readOnly);
89102

90-
var response = await _client.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, token);
103+
var response = await Client.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, token);
91104

92105
LogRetrieveResponse(response.StatusCode);
93106

@@ -119,9 +132,6 @@ private async Task<ISessionState> GetSessionDataAsync(string? sessionId, bool re
119132
/// </summary>
120133
private async Task SetOrReleaseSessionData(ISessionState? state, CancellationToken cancellationToken)
121134
{
122-
using var timeout = new CancellationTokenSource(_options.NetworkTimeout);
123-
using var cts = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token, cancellationToken);
124-
125135
using var req = new HttpRequestMessage { Method = HttpMethod.Put };
126136

127137
if (state is not null)
@@ -130,7 +140,7 @@ private async Task SetOrReleaseSessionData(ISessionState? state, CancellationTok
130140
req.Content = new SerializedSessionHttpContent(_serializer, state);
131141
}
132142

133-
using var response = await _client.SendAsync(req, cts.Token);
143+
using var response = await Client.SendAsync(req, cancellationToken);
134144

135145
LogCommitResponse(response.StatusCode);
136146

‎src/Microsoft.AspNetCore.SystemWebAdapters.SessionState/SessionConstants.Shared.cs

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState;
55

66
internal static class SessionConstants
77
{
8+
internal const string SessionClientName = "RemoteSessionHttpClient";
9+
810
public const string ReadOnlyHeaderName = "X-SystemWebAdapter-RemoteAppSession-ReadOnly";
911

1012
public const string SessionEndpointPath = "/systemweb-adapters/session";

‎src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/Authentication/AuthenticationConstants.Shared.cs

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication;
55

66
internal static class AuthenticationConstants
77
{
8+
internal const string AuthClientName = "RemoteAuthHttpClient";
9+
810
public const string ForwardedHostHeaderName = "x-forwarded-host";
911
public const string ForwardedProtoHeaderName = "x-forwarded-proto";
1012
public const string MigrationAuthenticateRequestHeaderName = "x-migration-authenticate";

‎src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/Authentication/RemoteAppAuthenticationClientOptions.cs

-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
54
using System.Collections.Generic;
65
using System.Collections.Immutable;
76
using System.ComponentModel.DataAnnotations;
@@ -37,13 +36,6 @@ public class RemoteAppAuthenticationClientOptions : AuthenticationSchemeOptions
3736
/// </summary>
3837
public ICollection<string> ResponseHeadersToForward { get; } = new HashSet<string>(DefaultResponseHeadersToForward);
3938

40-
/// <summary>
41-
/// The maximum time loading session state from the remote app
42-
/// or committing changes to it can take before timing out.
43-
/// </summary>
44-
[Required]
45-
public TimeSpan NetworkTimeout { get; set; } = TimeSpan.FromMinutes(1);
46-
4739
/// <summary>
4840
/// Gets or sets the endpoint on the remote app that provides remote authentication
4941
/// services. Requests to authenticate are sent to this endpoint.

‎src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/Authentication/RemoteAppAuthenticationExtensions.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ public static AuthenticationBuilder AddRemoteAppAuthentication(this Authenticati
5757

5858
authenticationBuilder.Services.AddScoped<IRemoteAppAuthenticationResultProcessor, RedirectUrlProcessor>();
5959
authenticationBuilder.Services.AddSingleton<IAuthenticationResultFactory, RemoteAppAuthenticationResultFactory>();
60-
authenticationBuilder.Services.AddHttpClient<IRemoteAppAuthenticationService, RemoteAppAuthenticationService>()
60+
authenticationBuilder.Services.AddHttpClient(AuthenticationConstants.AuthClientName)
6161
// Disable cookies in the HTTP client because the service will manage the cookie header directly
6262
.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { UseCookies = false, AllowAutoRedirect = false });
63-
63+
authenticationBuilder.Services.AddTransient<IRemoteAppAuthenticationService, RemoteAppAuthenticationService>();
6464
authenticationBuilder.Services.AddOptions<RemoteAppAuthenticationClientOptions>(scheme)
6565
.Configure(configureOptions ?? (_ => { }))
6666
.ValidateDataAnnotations();

‎src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/Authentication/RemoteAppAuthenticationService.cs

+7-4
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,20 @@ internal partial class RemoteAppAuthenticationService : IRemoteAppAuthentication
2929
private RemoteAppAuthenticationClientOptions? _options;
3030

3131
public RemoteAppAuthenticationService(
32-
HttpClient client,
32+
IHttpClientFactory httpClientFactory,
3333
IAuthenticationResultFactory resultFactory,
3434
IOptionsSnapshot<RemoteAppAuthenticationClientOptions> authOptions,
3535
IOptions<RemoteAppOptions> remoteAppOptions,
3636
ILogger<RemoteAppAuthenticationService> logger)
3737
{
38-
_client = client ?? throw new ArgumentNullException(nameof(client));
38+
_remoteAppOptions = remoteAppOptions?.Value ?? throw new ArgumentNullException(nameof(remoteAppOptions));
3939
_resultFactory = resultFactory ?? throw new ArgumentNullException(nameof(resultFactory));
4040
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
41-
_remoteAppOptions = remoteAppOptions?.Value ?? throw new ArgumentNullException(nameof(remoteAppOptions));
4241
_authOptionsSnapshot = authOptions ?? throw new ArgumentNullException(nameof(authOptions));
42+
43+
// Use the HttpClient supplied in options if one is present;
44+
// otherwise, generate a client with an IHttpClientFactory from DI
45+
_client = _remoteAppOptions.BackchannelHttpClient ?? httpClientFactory?.CreateClient(AuthenticationConstants.AuthClientName) ?? throw new ArgumentNullException(nameof(httpClientFactory));
4346
}
4447

4548
/// <summary>
@@ -51,7 +54,7 @@ public Task InitializeAsync(AuthenticationScheme scheme)
5154
// Finish initializing the http client here since the scheme won't be known
5255
// until the owning authentication handler is initialized.
5356
_options = _authOptionsSnapshot.Get(scheme.Name);
54-
_client.BaseAddress = new Uri(_remoteAppOptions.RemoteAppUrl, _options.AuthenticationEndpointPath);
57+
_client.BaseAddress = new Uri($"{_remoteAppOptions.RemoteAppUrl.ToString().TrimEnd('/')}{_options.AuthenticationEndpointPath}");
5558
_client.DefaultRequestHeaders.Add(_remoteAppOptions.ApiKeyHeader, _remoteAppOptions.ApiKey);
5659

5760
return Task.CompletedTask;

‎src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/RemoteAppOptions.Shared.cs

+7
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
#if NET6_0_OR_GREATER
66
using System.ComponentModel.DataAnnotations;
7+
using System.Net.Http;
78
#endif
89

910
namespace Microsoft.AspNetCore.SystemWebAdapters;
@@ -37,5 +38,11 @@ public class RemoteAppOptions
3738
/// </summary>
3839
[Required]
3940
public Uri RemoteAppUrl { get; set; } = null!;
41+
42+
/// <summary>
43+
/// Gets or sets an <see cref="HttpClient"/> to use for making requests to the remote app.
44+
/// A new <see cref="HttpClient"/> will be automatically generated if none is provided.
45+
/// </summary>
46+
public HttpClient? BackchannelHttpClient { get; set; }
4047
#endif
4148
}

0 commit comments

Comments
 (0)