Uh oh!
There was an error while loading. Please reload this page.
[tests] Use local servers for remaining networking tests - #12099
Conversation
PR #12022 migrated most on-device networking tests off live external URLs to the loopback `LocalHttpServer`/`LocalHttpsServer`, and #12058 handles `SslTest.HttpsShouldWork`. This migrates the remaining tests that still reached out to the public internet, which made them flaky when those endpoints were slow or unavailable: - `SslTest.SslWithinTasksShouldWork` (dotnet.microsoft.com) - `ProxyTest.QuoteInvalidQuoteUrlsShouldWork` (msftconnecttest.com) - keeps the `?query&foo|bar` unescaped-pipe scenario against `/ok`. - `AndroidMessageHandlerTests.ServerCertificateCustomValidationCallback_ApproveRequest` and `_RejectRequest` (www.microsoft.com) - `AndroidMessageHandlerTests.AndroidMessageHandlerSendsClientCertificate` (corefx-net-tls.azurewebsites.net) - `AndroidMessageHandlerIntegrationTests.GetString_Many` (google.com) - `HttpClientHandlerTestBase.Disposed` (google.com) - `WebSocketTests.TestSocketConnection` (echo.websocket.org) - was `[Ignore]`d because the echo server no longer exists; re-enabled against a local server. To support these, `LocalTestServers.cs` gains: - Opt-in client-certificate (mutual TLS) support on `LocalHttpsServer` plus an `/echo-client-certificate` endpoint that returns the client certificate the server received. - A new loopback `LocalWebSocketServer` that performs the WebSocket upgrade handshake and echoes messages back. Also cleaned up cosmetic external hostnames in `CancelRequestViaProxy` (request never connects; goes through a dead proxy) and the `DisallowAutoRedirect` redirect target (never followed). The `http://10.255.255.1` cancellation/timeout tests intentionally use an unroutable address and are left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 289a099a-278f-432b-b2b4-aac036e6e2dc
The local WebSocket test server computes the Sec-WebSocket-Accept value using SHA-1, as mandated by the WebSocket handshake (RFC 6455 §1.3). This is a protocol requirement, not a security-sensitive choice, but CA5350 (treated as an error in the test build) flagged it and broke the Mono.Android.NET-Tests APK build. Suppress CA5350 around the SHA-1 usage with a justification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 528dc903-4f7c-4a75-b74b-da0fbfe23f01
There was a problem hiding this comment.
Pull request overview
This PR reduces flakiness in on-device networking tests by replacing remaining dependencies on public internet endpoints with loopback LocalHttpServer/LocalHttpsServer, and by adding new local server functionality (mutual-TLS client cert echo + WebSocket echo) in the existing test server infrastructure.
Changes:
- Migrate remaining tests from external hosts (e.g., microsoft.com/google.com/echo.websocket.org) to local loopback servers.
- Extend
LocalHttpsServerto support client-certificate scenarios via a new/echo-client-certificateendpoint. - Add
LocalWebSocketServerto re-enable WebSocket echo coverage without external dependencies.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/LocalTestServers.cs | Makes HTTP handler overridable, adds mutual-TLS client cert echo endpoint, and introduces a local WebSocket echo server. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs | Replaces microsoft.com/corefx TLS endpoints with LocalHttpsServer and validates behavior against local responses. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerIntegrationTests.cs | Switches multi-request tests and cosmetic URLs to loopback/local addresses. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidHandlerTestBase.cs | Updates disposed-handler networking test to target loopback server instead of google.com. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/WebSocketTests.cs | Re-enables WebSocket test using LocalWebSocketServer instead of a defunct public echo server. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/SslTest.cs | Moves SslWithinTasksShouldWork off dotnet.microsoft.com onto local HTTPS server. |
| tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/ProxyTest.cs | Moves invalid-URL quoting test off msftconnecttest.com onto local HTTP server. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 3
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Addresses PR review feedback now that these tests target loopback servers: - ProxyTest.QuoteInvalidQuoteUrlsShouldWork: drop the ConnectFailure/ NameResolutionFailure/Timeout Assert.Ignore path; loopback failures are real regressions and should fail the test. - SslTest.SslWithinTasksShouldWork: rethrow the WebException instead of ignoring 'network failure' (loopback is always reachable). - HttpClientHandlerTestBase.Disposed: call GetAsync directly instead of via ConnectIgnoreFailure so an unexpected exception type isn't masked. - LocalHttpsServer: rename requestClientCertificate -> clientCertificateRequired to match AuthenticateAsServerAsync's clientCertificateRequired argument. Not applied: adding AssertNoUnhandledExceptions() to ServerCertificateCustomValidationCallback_RejectRequest. When the client rejects the server certificate the TLS handshake is aborted, so the server's AuthenticateAsServerAsync throws before handlerCompleted becomes true and the exception is recorded; asserting no handler exceptions would fail that test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 528dc903-4f7c-4a75-b74b-da0fbfe23f01
simonrozsival
commented
Jul 14, 2026
Also addressed the two review comments from #12098 (same code) in f620ce0:
|
simonrozsival
commented
Jul 14, 2026
/review |
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Android PR Review — ⚠️ Looks good, minor suggestions
Thorough migration of the remaining on-device networking tests off public endpoints onto loopback LocalHttpServer/LocalHttpsServer/LocalWebSocketServer. The approach is sound: removing external dependencies eliminates a real class of flakiness, the self-signed cert plumbing (SAN, EKU, mutual-TLS requestClientCertificate) is correct, and the AssertNoUnhandledExceptions pattern surfaces server-side failures that would otherwise be swallowed. Verifying end-to-end on desktop .NET before device runs is the right call.
Findings (all 💡 suggestions — nothing blocking)
- Duplication —
LocalWebSocketServerre-implements theTcpListener/accept-loop/Disposeboilerplate and the CRLFCRLF header-reading loop already present inLocalHttpServer. Consider hoisting both into the shared base. (2 inline comments) AssertNoUnhandledExceptionstiming — for the WebSocket test the server handler is still running when the assert fires (client closes abruptly), so a late handler exception could be missed. (inline comment)- Stale
InetAccesscategory —ProxyTest,SslTest.SslWithinTasksShouldWork, and the re-enabledWebSocketTests.TestSocketConnectionno longer touch the internet, yet keep[Category ("InetAccess")], which excludes them underEnableLLVMruns. (inline comment)
CI
Status is pending (no checks reported yet) — not a blocker, but confirm the on-device suites go green before merge, since the desktop verification doesn't exercise the Android AndroidMessageHandler TLS/client-cert paths.
Nice cleanup overall. 👍
Generated by Android PR Reviewer for #12099 · 156.2 AIC · ⌖ 13.4 AIC · ⊞ 6.8K
Comment /review to run again
| { | ||
| const string WebSocketGuid = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"; | ||
| readonly TcpListener listener; |
There was a problem hiding this comment.
🤖 💡 Code organization — LocalWebSocketServer re-implements the TcpListener/Port/Uri/Url/StartListening/Dispose/AcceptLoop boilerplate that already exists in LocalHttpServer. Consider hoisting the listener + accept-loop plumbing into the shared LocalTestServer base (with an abstract HandleClient) so the two servers don't drift out of sync.
(Rule: Avoid duplication)
| } | ||
| } | ||
| static async Task<string> ReadWebSocketKeyAsync (Stream stream) |
There was a problem hiding this comment.
🤖 💡 Code organization — This byte-at-a-time header-reading loop is nearly identical to ReadRequestAsync in LocalHttpServer (same 64 KB cap, same CRLFCRLF state machine). Extracting a shared ReadHeadersAsync (Stream) helper would remove the duplication and keep both parsers consistent.
(Rule: Avoid duplication)
| using var server = LocalWebSocketServer.Start (); | ||
| var result = CustomWebSocket.Connect (server.Url, messageBytes).Result; | ||
| Assert.AreEqual (result, testMessage, $"Socket test failed. Expected: {testMessage}, Received: {result}"); | ||
| server.AssertNoUnhandledExceptions (); |
There was a problem hiding this comment.
🤖 💡 Testing — After .Result returns, the client ClientWebSocket is already disposed (Connect closes it), so it tears down the connection without a WebSocket close handshake. That means the server's HandleClient is still running (blocked in EchoLoop's ReceiveAsync) when AssertNoUnhandledExceptions () executes here — a genuine server-side handler failure could be recorded after this assert and silently missed. Consider giving the server a brief drain/join before asserting (e.g. signal completion from HandleClient), or document that the assert is best-effort for this server.
(Rule: Verify async handlers before asserting)
| var messageBytes = CustomWebSocket.GetBytes (testMessage); | ||
| CustomWebSocket.BytesSize = messageBytes.Length; | ||
| var result = CustomWebSocket.Connect ("ws://echo.websocket.org", messageBytes).Result; | ||
| using var server = LocalWebSocketServer.Start (); |
There was a problem hiding this comment.
🤖 💡 Testing — Now that this test talks to a loopback LocalWebSocketServer instead of echo.websocket.org, the [Category ("InetAccess")] on the test is arguably stale. InetAccess is excluded on EnableLLVM runs (see TestInstrumentation.cs), so keeping it means this newly re-enabled test still won't run under LLVM even though it no longer needs the internet. Consider dropping the InetAccess category (the same applies to ProxyTest/SslTest.SslWithinTasksShouldWork, which are also loopback-only now).
(Rule: Keep test categories accurate)
Context
Following up on #12022 (which migrated most on-device networking tests off live external URLs to the loopback
LocalHttpServer/LocalHttpsServer) and #12058 (which handlesSslTest.HttpsShouldWork), a few on-device tests were still reaching out to the public internet. Those requests made the tests flaky whenever the external endpoints were slow, rate-limited, or unavailable.This migrates the remaining tests to local loopback servers.
Tests migrated
SslTest.SslWithinTasksShouldWorkdotnet.microsoft.comProxyTest.QuoteInvalidQuoteUrlsShouldWorkmsftconnecttest.comAndroidMessageHandlerTests.ServerCertificateCustomValidationCallback_ApproveRequestwww.microsoft.comAndroidMessageHandlerTests.ServerCertificateCustomValidationCallback_RejectRequestwww.microsoft.comAndroidMessageHandlerTests.AndroidMessageHandlerSendsClientCertificatecorefx-net-tls.azurewebsites.netAndroidMessageHandlerIntegrationTests.GetString_Manygoogle.comHttpClientHandlerTestBase.Disposedgoogle.comWebSocketTests.TestSocketConnectionecho.websocket.org(was[Ignore]d)ProxyTestkeeps its original point — an unescaped|in the query string (?query&foo|bar) — now exercised against the local/okendpoint.WebSocketTestswas previously disabled because the public echo server no longer exists; it is re-enabled against a local server.Test infrastructure (
LocalTestServers.cs)LocalHttpsServer.Start (requestClientCertificate: true)now requests a client certificate during the TLS handshake, and a new/echo-client-certificateendpoint returns the certificate the server received (base64 DER). This replaces the externalEchoClientCertificate.ashxendpoint.LocalWebSocketServer: a loopback WebSocket echo server that performs the upgrade handshake and echoes messages back (usesWebSocket.CreateFromStreamafter a manual101 Switching Protocolsresponse).Notes
HttpsShouldWorkis intentionally left to Use a local HTTPS server inSslTest.HttpsShouldWork#12058.http://10.255.255.1cancellation/timeout tests deliberately use an unroutable address (not the public internet) and are unchanged.CancelRequestViaProxy's base address behind a dead proxy, andDisallowAutoRedirect's redirect target) were pointed atlocalhost.The new server logic (HTTP, HTTPS, mutual-TLS echo, and WebSocket echo) was verified end-to-end against
HttpClientandClientWebSocketon desktop .NET.