Skip to content

Commit 60264bb

Browse files
baywetCopilot
baywet
and
Copilot
authored
tests: refactors redirection tests to avoid race condition (#14342)
while working on #14340 I noticed some flaky unit tests are you can see in https://github.com/microsoft/semantic-kernel/actions/runs/32982261836/job/98221741513?pr=14340. This PR refactors the involved tests so they don't run into each other --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent b4b0ae7 commit 60264bb

3 files changed

Lines changed: 129 additions & 71 deletions

File tree

‎dotnet/src/Plugins/Plugins.UnitTests/Core/HttpPluginTests.cs‎

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -137,47 +137,16 @@ public async Task ItThrowsInvalidOperationExceptionForInvalidDomainAsync()
137137
public async Task ItDoesNotFollowRedirectsAsync()
138138
{
139139
// Arrange - start a local server that always returns a 302 redirect
140-
using var listener = new System.Net.HttpListener();
141-
var port = new Random().Next(49152, 65535);
142-
listener.Prefixes.Add($"http://localhost:{port}/");
143-
listener.Start();
144-
bool redirectTargetContacted = false;
145-
146-
_ = Task.Run(async () =>
147-
{
148-
while (listener.IsListening)
149-
{
150-
try
151-
{
152-
var ctx = await listener.GetContextAsync();
153-
if (ctx.Request.Url!.AbsolutePath == "/start")
154-
{
155-
ctx.Response.StatusCode = 302;
156-
ctx.Response.RedirectLocation = $"http://localhost:{port}/secret";
157-
ctx.Response.Close();
158-
}
159-
else if (ctx.Request.Url.AbsolutePath == "/secret")
160-
{
161-
redirectTargetContacted = true;
162-
ctx.Response.StatusCode = 200;
163-
ctx.Response.Close();
164-
}
165-
}
166-
catch (ObjectDisposedException) { break; }
167-
catch (System.Net.HttpListenerException) { break; }
168-
}
169-
});
140+
await using var server = new RedirectLoopbackServer("secret", "text/plain", []);
170141

171142
var plugin = new HttpPlugin()
172143
{
173-
AllowedDomains = ["localhost"]
144+
AllowedDomains = [server.BaseUri.Host]
174145
};
175146

176147
// Act & Assert - the plugin should throw because 302 is a non-success status
177-
await Assert.ThrowsAsync<HttpOperationException>(() => plugin.GetAsync($"http://localhost:{port}/start"));
178-
Assert.False(redirectTargetContacted, "The redirect target should not have been contacted.");
179-
180-
listener.Stop();
148+
await Assert.ThrowsAsync<HttpOperationException>(() => plugin.GetAsync(new Uri(server.BaseUri, "start").AbsoluteUri));
149+
Assert.False(server.RedirectTargetContacted, "The redirect target should not have been contacted.");
181150
}
182151

183152
private Mock<HttpMessageHandler> CreateMock()

‎dotnet/src/Plugins/Plugins.UnitTests/Support/RedirectLoopbackServer.cs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
using System;
4+
using System.IO;
5+
using System.Net;
6+
using System.Net.Sockets;
7+
using System.Text;
8+
using System.Threading;
9+
using System.Threading.Tasks;
10+
11+
namespace SemanticKernel.Plugins.UnitTests;
12+
13+
internal sealed class RedirectLoopbackServer : IAsyncDisposable
14+
{
15+
private readonly TcpListener _listener;
16+
private readonly CancellationTokenSource _cancellationTokenSource = new();
17+
private readonly Task _serverTask;
18+
private readonly string _redirectTargetPath;
19+
private readonly string _targetContentType;
20+
private readonly byte[] _targetContent;
21+
private int _redirectTargetContacted;
22+
23+
public RedirectLoopbackServer(string redirectTargetPath, string targetContentType, byte[] targetContent)
24+
{
25+
this._redirectTargetPath = redirectTargetPath.StartsWith("/", StringComparison.Ordinal) ? redirectTargetPath : $"/{redirectTargetPath}";
26+
this._targetContentType = targetContentType;
27+
this._targetContent = targetContent;
28+
29+
this._listener = new TcpListener(IPAddress.Loopback, 0);
30+
this._listener.Start();
31+
var endpoint = (IPEndPoint)this._listener.LocalEndpoint;
32+
this.BaseUri = new Uri($"http://{endpoint.Address}:{endpoint.Port}/");
33+
this._serverTask = this.RunAsync();
34+
}
35+
36+
public Uri BaseUri { get; }
37+
38+
public bool RedirectTargetContacted => Volatile.Read(ref this._redirectTargetContacted) != 0;
39+
40+
public async ValueTask DisposeAsync()
41+
{
42+
this._cancellationTokenSource.Cancel();
43+
this._listener.Stop();
44+
await this._serverTask.ConfigureAwait(false);
45+
this._cancellationTokenSource.Dispose();
46+
}
47+
48+
private async Task RunAsync()
49+
{
50+
while (!this._cancellationTokenSource.IsCancellationRequested)
51+
{
52+
TcpClient client;
53+
try
54+
{
55+
client = await this._listener.AcceptTcpClientAsync(this._cancellationTokenSource.Token).ConfigureAwait(false);
56+
}
57+
catch (OperationCanceledException)
58+
{
59+
break;
60+
}
61+
catch (ObjectDisposedException) when (this._cancellationTokenSource.IsCancellationRequested)
62+
{
63+
break;
64+
}
65+
catch (SocketException) when (this._cancellationTokenSource.IsCancellationRequested)
66+
{
67+
break;
68+
}
69+
70+
using (client)
71+
{
72+
try
73+
{
74+
await this.HandleRequestAsync(client, this._cancellationTokenSource.Token).ConfigureAwait(false);
75+
}
76+
catch (OperationCanceledException) when (this._cancellationTokenSource.IsCancellationRequested)
77+
{
78+
break;
79+
}
80+
catch (ObjectDisposedException) when (this._cancellationTokenSource.IsCancellationRequested)
81+
{
82+
break;
83+
}
84+
catch (IOException)
85+
{
86+
// Ignore client disconnects / truncated requests to keep the test helper stable.
87+
}
88+
catch (SocketException) when (this._cancellationTokenSource.IsCancellationRequested)
89+
{
90+
break;
91+
}
92+
}
93+
}
94+
}
95+
96+
private async Task HandleRequestAsync(TcpClient client, CancellationToken cancellationToken)
97+
{
98+
var stream = client.GetStream();
99+
using var reader = new StreamReader(stream, Encoding.ASCII, detectEncodingFromByteOrderMarks: false, bufferSize: 1024, leaveOpen: true);
100+
var requestLine = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
101+
string? header;
102+
do
103+
{
104+
header = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
105+
}
106+
while (!string.IsNullOrEmpty(header));
107+
108+
var requestTarget = requestLine?.Split(' ')[1];
109+
if (requestTarget == "/start")
110+
{
111+
var response = $"HTTP/1.1 302 Found\r\nLocation: {new Uri(this.BaseUri, this._redirectTargetPath.TrimStart('/'))}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
112+
await stream.WriteAsync(Encoding.ASCII.GetBytes(response), cancellationToken).ConfigureAwait(false);
113+
return;
114+
}
115+
116+
Interlocked.Exchange(ref this._redirectTargetContacted, 1);
117+
var responseHeaders = $"HTTP/1.1 200 OK\r\nContent-Type: {this._targetContentType}\r\nContent-Length: {this._targetContent.Length}\r\nConnection: close\r\n\r\n";
118+
await stream.WriteAsync(Encoding.ASCII.GetBytes(responseHeaders), cancellationToken).ConfigureAwait(false);
119+
await stream.WriteAsync(this._targetContent, cancellationToken).ConfigureAwait(false);
120+
}
121+
}

‎dotnet/src/Plugins/Plugins.UnitTests/Web/WebFileDownloadPluginTests.cs‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -82,59 +82,27 @@ public async Task DownloadToFileFailsForInvalidDomainAsync()
8282
public async Task DownloadToFileDoesNotFollowRedirectsAsync()
8383
{
8484
// Arrange - start a local server that returns a 302 redirect
85-
using var listener = new System.Net.HttpListener();
86-
var port = new Random().Next(49152, 65535);
87-
listener.Prefixes.Add($"http://localhost:{port}/");
88-
listener.Start();
89-
bool redirectTargetContacted = false;
90-
91-
_ = Task.Run(async () =>
92-
{
93-
while (listener.IsListening)
94-
{
95-
try
96-
{
97-
var ctx = await listener.GetContextAsync();
98-
if (ctx.Request.Url!.AbsolutePath == "/start")
99-
{
100-
ctx.Response.StatusCode = 302;
101-
ctx.Response.RedirectLocation = $"http://localhost:{port}/secret.png";
102-
ctx.Response.Close();
103-
}
104-
else if (ctx.Request.Url.AbsolutePath == "/secret.png")
105-
{
106-
redirectTargetContacted = true;
107-
ctx.Response.StatusCode = 200;
108-
ctx.Response.ContentType = "image/png";
109-
ctx.Response.OutputStream.Write(new byte[] { 0x89, 0x50, 0x4E, 0x47 });
110-
ctx.Response.Close();
111-
}
112-
}
113-
catch (ObjectDisposedException) { break; }
114-
catch (System.Net.HttpListenerException) { break; }
115-
}
116-
});
85+
await using var server = new RedirectLoopbackServer("secret.png", "image/png", [0x89, 0x50, 0x4E, 0x47]);
11786

11887
var folderPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
11988
var filePath = Path.Combine(folderPath, "file.png");
12089
Directory.CreateDirectory(folderPath);
12190

12291
var webFileDownload = new WebFileDownloadPlugin()
12392
{
124-
AllowedDomains = ["localhost"],
93+
AllowedDomains = [server.BaseUri.Host],
12594
AllowedFolders = [folderPath]
12695
};
12796

12897
try
12998
{
13099
// Act & Assert - the plugin should throw because 302 is a non-success status
131-
await Assert.ThrowsAsync<HttpOperationException>(() => webFileDownload.DownloadToFileAsync(new Uri($"http://localhost:{port}/start"), filePath));
132-
Assert.False(redirectTargetContacted, "The redirect target should not have been contacted.");
100+
await Assert.ThrowsAsync<HttpOperationException>(() => webFileDownload.DownloadToFileAsync(new Uri(server.BaseUri, "start"), filePath));
101+
Assert.False(server.RedirectTargetContacted, "The redirect target should not have been contacted.");
133102
Assert.False(Path.Exists(filePath));
134103
}
135104
finally
136105
{
137-
listener.Stop();
138106
if (Path.Exists(folderPath))
139107
{
140108
Directory.Delete(folderPath, true);

0 commit comments

Comments
 (0)