Skip to content

Commit 1b7b020

Browse files
Update OpenAPI HTTP client defaults (#14293)
Updates OpenAPI HTTP client defaults and related test coverage. --------- Copilot-Session: da844777-e333-41e3-a18e-76794e1119d9
1 parent c1afc86 commit 1b7b020

7 files changed

Lines changed: 159 additions & 19 deletions

File tree

‎dotnet/src/Functions/Functions.OpenApi.Extensions/Extensions/ApiManifestKernelExtensions.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
using Microsoft.OpenApi.ApiManifest;
1515
using Microsoft.OpenApi.Readers;
1616
using Microsoft.OpenApi.Services;
17-
using Microsoft.SemanticKernel.Http;
1817
using Microsoft.SemanticKernel.Plugins.OpenApi;
1918
using Microsoft.SemanticKernel.Plugins.OpenApi.Extensions;
2019

@@ -103,7 +102,7 @@ public static async Task<KernelPlugin> CreatePluginFromApiManifestAsync(
103102
KernelVerify.ValidPluginName(pluginName, kernel.Plugins);
104103

105104
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
106-
var httpClient = HttpClientProvider.GetHttpClient(pluginParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
105+
var httpClient = OpenApiKernelPluginFactory.GetHttpClient(pluginParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
107106
#pragma warning restore CA2000
108107

109108
if (!File.Exists(filePath))
@@ -187,7 +186,7 @@ await DocumentLoader.LoadDocumentFromUriAsStreamAsync(new Uri(apiDescriptionUrl)
187186
};
188187

189188
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
190-
var operationRunnerHttpClient = HttpClientProvider.GetHttpClient(openApiFunctionExecutionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
189+
var operationRunnerHttpClient = OpenApiKernelPluginFactory.GetHttpClient(openApiFunctionExecutionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
191190
#pragma warning restore CA2000
192191

193192
var runner = new RestApiOperationRunner(

‎dotnet/src/Functions/Functions.OpenApi.Extensions/Extensions/CopilotAgentPluginKernelExtensions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public static async Task<KernelPlugin> CreatePluginFromCopilotAgentPluginAsync(
6565
KernelVerify.ValidPluginName(pluginName, kernel.Plugins);
6666

6767
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
68-
var httpClient = HttpClientProvider.GetHttpClient(pluginParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
68+
var httpClient = OpenApiKernelPluginFactory.GetHttpClient(pluginParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
6969
#pragma warning restore CA2000
7070

7171
if (!File.Exists(filePath))
@@ -158,7 +158,7 @@ await DocumentLoader.LoadDocumentFromUriAsStreamAsync(parsedDescriptionUrl,
158158
};
159159

160160
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
161-
var operationRunnerHttpClient = HttpClientProvider.GetHttpClient(openApiFunctionExecutionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
161+
var operationRunnerHttpClient = OpenApiKernelPluginFactory.GetHttpClient(openApiFunctionExecutionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
162162
#pragma warning restore CA2000
163163
static IDictionary<string, string>? CopilotAgentPluginHeadersFactory(RestApiOperation operation, IDictionary<string, object?> arguments, RestApiOperationRunOptions? options)
164164
{

‎dotnet/src/Functions/Functions.OpenApi/Extensions/OpenApiFunctionExecutionParameters.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public class OpenApiFunctionExecutionParameters
1717
/// <summary>
1818
/// HttpClient to use for sending HTTP requests.
1919
/// </summary>
20+
/// <remarks>
21+
/// The default client does not follow redirects so that URL validation cannot be bypassed.
22+
/// Custom clients should also be configured with <c>AllowAutoRedirect = false</c>.
23+
/// </remarks>
2024
public HttpClient? HttpClient { get; set; }
2125

2226
/// <summary>

‎dotnet/src/Functions/Functions.OpenApi/Extensions/OpenApiKernelExtensions.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using Microsoft.Extensions.DependencyInjection;
99
using Microsoft.Extensions.Logging;
1010
using Microsoft.Extensions.Logging.Abstractions;
11-
using Microsoft.SemanticKernel.Http;
1211
using Microsoft.SemanticKernel.Plugins.OpenApi;
1312

1413
namespace Microsoft.SemanticKernel;
@@ -120,7 +119,7 @@ public static async Task<KernelPlugin> CreatePluginFromOpenApiAsync(
120119
KernelVerify.ValidPluginName(pluginName, kernel.Plugins);
121120

122121
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
123-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
122+
var httpClient = OpenApiKernelPluginFactory.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
124123
#pragma warning restore CA2000
125124

126125
ILoggerFactory loggerFactory = executionParameters?.LoggerFactory ?? kernel.LoggerFactory;
@@ -159,7 +158,7 @@ public static async Task<KernelPlugin> CreatePluginFromOpenApiAsync(
159158
KernelVerify.ValidPluginName(pluginName, kernel.Plugins);
160159

161160
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
162-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
161+
var httpClient = OpenApiKernelPluginFactory.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
163162
#pragma warning restore CA2000
164163

165164
ILoggerFactory loggerFactory = executionParameters?.LoggerFactory ?? kernel.LoggerFactory;
@@ -202,7 +201,7 @@ public static async Task<KernelPlugin> CreatePluginFromOpenApiAsync(
202201
KernelVerify.ValidPluginName(pluginName, kernel.Plugins);
203202

204203
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
205-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
204+
var httpClient = OpenApiKernelPluginFactory.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
206205
#pragma warning restore CA2000
207206

208207
var openApiSpec = await DocumentLoader.LoadDocumentFromStreamAsync(stream, cancellationToken).ConfigureAwait(false);
@@ -236,7 +235,7 @@ public static KernelPlugin CreatePluginFromOpenApi(
236235
KernelVerify.ValidPluginName(pluginName, kernel.Plugins);
237236

238237
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
239-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
238+
var httpClient = OpenApiKernelPluginFactory.GetHttpClient(executionParameters?.HttpClient ?? kernel.Services.GetService<HttpClient>());
240239
#pragma warning restore CA2000
241240

242241
return OpenApiKernelPluginFactory.CreateOpenApiPlugin(

‎dotnet/src/Functions/Functions.OpenApi/OpenApiKernelPluginFactory.cs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public static async Task<KernelPlugin> CreateFromOpenApiAsync(
4141
KernelVerify.ValidPluginName(pluginName);
4242

4343
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
44-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient);
44+
var httpClient = GetHttpClient(executionParameters?.HttpClient);
4545
#pragma warning restore CA2000
4646

4747
var loggerFactory = executionParameters?.LoggerFactory;
@@ -78,7 +78,7 @@ public static async Task<KernelPlugin> CreateFromOpenApiAsync(
7878
KernelVerify.ValidPluginName(pluginName);
7979

8080
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
81-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient);
81+
var httpClient = GetHttpClient(executionParameters?.HttpClient);
8282
#pragma warning restore CA2000
8383

8484
var loggerFactory = executionParameters?.LoggerFactory;
@@ -119,7 +119,7 @@ public static async Task<KernelPlugin> CreateFromOpenApiAsync(
119119
KernelVerify.ValidPluginName(pluginName);
120120

121121
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
122-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient);
122+
var httpClient = GetHttpClient(executionParameters?.HttpClient);
123123
#pragma warning restore CA2000
124124

125125
var openApiSpec = await DocumentLoader.LoadDocumentFromStreamAsync(stream, cancellationToken).ConfigureAwait(false);
@@ -148,7 +148,7 @@ public static KernelPlugin CreateFromOpenApi(
148148
KernelVerify.ValidPluginName(pluginName);
149149

150150
#pragma warning disable CA2000 // Dispose objects before losing scope. No need to dispose the Http client here. It can either be an internal client using NonDisposableHttpClientHandler or an external client managed by the calling code, which should handle its disposal.
151-
var httpClient = HttpClientProvider.GetHttpClient(executionParameters?.HttpClient);
151+
var httpClient = GetHttpClient(executionParameters?.HttpClient);
152152
#pragma warning restore CA2000
153153

154154
return CreateOpenApiPlugin(
@@ -194,6 +194,12 @@ internal static async Task<KernelPlugin> CreateOpenApiPluginAsync(
194194
loggerFactory: loggerFactory);
195195
}
196196

197+
/// <summary>
198+
/// Returns the supplied HTTP client or the non-redirecting OpenAPI default.
199+
/// </summary>
200+
internal static HttpClient GetHttpClient(HttpClient? httpClient = null) =>
201+
httpClient ?? HttpClientProvider.GetNonRedirectingHttpClient();
202+
197203
/// <summary>
198204
/// Creates a plugin from an OpenAPI specification.
199205
/// </summary>

‎dotnet/src/Functions/Functions.OpenApi/RestApiOperationServerUrlValidationOptions.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,9 @@ namespace Microsoft.SemanticKernel.Plugins.OpenApi;
3131
/// used to opt specific intranet hosts back in.
3232
/// </para>
3333
/// <para>
34-
/// <b>Known limitation:</b> URL validation is performed before the HTTP request is sent. If the
35-
/// <see cref="System.Net.Http.HttpClient"/> used to invoke the plugin has automatic redirect
36-
/// following enabled (the default), an attacker that controls a public host may redirect the
37-
/// request to a private address. Configure your <see cref="System.Net.Http.HttpClient"/> with
38-
/// <c>AllowAutoRedirect = false</c> when consuming OpenAPI documents from untrusted sources.
34+
/// The OpenAPI plugin's default HTTP client does not follow redirects, preventing redirects
35+
/// from bypassing this validation. Caller-provided HTTP clients must also be configured with
36+
/// <c>AllowAutoRedirect = false</c> to preserve this guarantee.
3937
/// </para>
4038
/// </remarks>
4139
[Experimental("SKEXP0040")]

‎dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiKernelPluginFactoryTests.cs‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
using System.Diagnostics.CodeAnalysis;
66
using System.IO;
77
using System.Linq;
8+
using System.Net;
89
using System.Net.Http;
910
using System.Net.Mime;
11+
using System.Net.Sockets;
1012
using System.Text;
1113
using System.Text.Json.Nodes;
1214
using System.Threading;
@@ -51,6 +53,57 @@ public OpenApiKernelPluginFactoryTests()
5153
this._openApiDocument = ResourcePluginsProvider.LoadFromResource("documentV2_0.json");
5254
}
5355

56+
[Fact]
57+
public async Task ItDoesNotFollowRedirectsByDefaultAsync()
58+
{
59+
// Arrange
60+
await using var server = new RedirectLoopbackServer();
61+
var serverUrl = server.BaseUri.AbsoluteUri;
62+
63+
var operation = new RestApiOperation(
64+
id: "redirect",
65+
servers: [new RestApiServer(serverUrl)],
66+
path: "start",
67+
method: HttpMethod.Get,
68+
description: "redirect test",
69+
parameters: [],
70+
responses: new Dictionary<string, RestApiExpectedResponse>(),
71+
securityRequirements: []);
72+
var specification = new RestApiSpecification(new RestApiInfo(), null, [operation]);
73+
var executionParameters = new OpenApiFunctionExecutionParameters
74+
{
75+
ServerUrlValidationOptions = new RestApiOperationServerUrlValidationOptions
76+
{
77+
AllowedBaseUrls = [new Uri(serverUrl)]
78+
}
79+
};
80+
var plugin = OpenApiKernelPluginFactory.CreateFromOpenApi("redirectPlugin", specification, executionParameters);
81+
var kernel = new Kernel();
82+
83+
// Act
84+
await Assert.ThrowsAsync<HttpOperationException>(() => kernel.InvokeAsync(plugin["redirect"]));
85+
86+
// Assert
87+
Assert.False(server.RedirectTargetContacted);
88+
}
89+
90+
[Fact]
91+
public async Task ItDoesNotFollowRedirectsWhenLoadingOpenApiDocumentsByDefaultAsync()
92+
{
93+
// Arrange
94+
await using var server = new RedirectLoopbackServer();
95+
96+
// Act
97+
await Assert.ThrowsAsync<HttpOperationException>(() =>
98+
OpenApiKernelPluginFactory.CreateFromOpenApiAsync(
99+
"redirectedDocumentPlugin",
100+
new Uri(server.BaseUri, "start"),
101+
this._executionParameters));
102+
103+
// Assert
104+
Assert.False(server.RedirectTargetContacted);
105+
}
106+
54107
[Fact]
55108
public async Task ItCanIncludeOpenApiOperationParameterTypesIntoFunctionParametersViewAsync()
56109
{
@@ -771,6 +824,87 @@ private static void AssertPayloadParameters(KernelPlugin plugin, string function
771824
Assert.Equal("content_type", function.Metadata.Parameters[1].Name);
772825
}
773826

827+
private sealed class RedirectLoopbackServer : IAsyncDisposable
828+
{
829+
private readonly TcpListener _listener;
830+
private readonly CancellationTokenSource _cancellationTokenSource = new();
831+
private readonly Task _serverTask;
832+
private int _redirectTargetContacted;
833+
834+
public RedirectLoopbackServer()
835+
{
836+
this._listener = new TcpListener(IPAddress.Loopback, 0);
837+
this._listener.Start();
838+
var endpoint = (IPEndPoint)this._listener.LocalEndpoint;
839+
this.BaseUri = new Uri($"http://{endpoint.Address}:{endpoint.Port}/");
840+
this._serverTask = this.RunAsync();
841+
}
842+
843+
public Uri BaseUri { get; }
844+
845+
public bool RedirectTargetContacted => Volatile.Read(ref this._redirectTargetContacted) != 0;
846+
847+
public async ValueTask DisposeAsync()
848+
{
849+
this._cancellationTokenSource.Cancel();
850+
this._listener.Stop();
851+
await this._serverTask.ConfigureAwait(false);
852+
this._cancellationTokenSource.Dispose();
853+
}
854+
855+
private async Task RunAsync()
856+
{
857+
while (!this._cancellationTokenSource.IsCancellationRequested)
858+
{
859+
TcpClient client;
860+
try
861+
{
862+
client = await this._listener.AcceptTcpClientAsync(this._cancellationTokenSource.Token).ConfigureAwait(false);
863+
}
864+
catch (OperationCanceledException)
865+
{
866+
break;
867+
}
868+
catch (SocketException) when (this._cancellationTokenSource.IsCancellationRequested)
869+
{
870+
break;
871+
}
872+
873+
using (client)
874+
{
875+
await this.HandleRequestAsync(client, this._cancellationTokenSource.Token).ConfigureAwait(false);
876+
}
877+
}
878+
}
879+
880+
private async Task HandleRequestAsync(TcpClient client, CancellationToken cancellationToken)
881+
{
882+
var stream = client.GetStream();
883+
using var reader = new StreamReader(stream, Encoding.ASCII, detectEncodingFromByteOrderMarks: false, bufferSize: 1024, leaveOpen: true);
884+
var requestLine = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
885+
string? header;
886+
do
887+
{
888+
header = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
889+
}
890+
while (!string.IsNullOrEmpty(header));
891+
892+
var requestTarget = requestLine?.Split(' ')[1];
893+
string response;
894+
if (requestTarget == "/start")
895+
{
896+
response = $"HTTP/1.1 302 Found\r\nLocation: {new Uri(this.BaseUri, "target")}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
897+
}
898+
else
899+
{
900+
Interlocked.Exchange(ref this._redirectTargetContacted, 1);
901+
response = "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}";
902+
}
903+
904+
await stream.WriteAsync(Encoding.ASCII.GetBytes(response), cancellationToken).ConfigureAwait(false);
905+
}
906+
}
907+
774908
private sealed class FakePlugin
775909
{
776910
public string? ParameterValueFakeMethodCalledWith { get; private set; }

0 commit comments

Comments
 (0)