Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 901af62

Browse files
hughbestephentoub
hughbe
authored and
stephentoub
committed
Fix several GetClientCertificate tests for the managed implementation of HttpListener (#19757)
* Perform argument validation on the return of BeginGetClientCertificate * Move non-Windows/managed implementation specific code for certificates to a platform abstraction layer * Preliminary PR feedback, rebase to fix a merge conflict * Call the actually simple method that doesn't have tracing etc. etc. in BeginGetClientCertificate
1 parent 6e9eefa commit 901af62

File tree

6 files changed

+104
-98
lines changed

6 files changed

+104
-98
lines changed

‎src/System.Net.HttpListener/src/System.Net.HttpListener.csproj‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
<Compile Include="System\Net\HttpListenerContext.cs" />
6161
<Compile Include="System\Net\HttpListenerException.cs" />
6262
<Compile Include="System\Net\HttpListenerResponse.cs" />
63+
<Compile Include="System\Net\ListenerClientCertState.cs" />
6364
<Compile Include="System\Net\NetEventSource.HttpListener.cs" />
6465
<Compile Include="System\Net\WebSockets\HttpListenerWebSocketContext.cs" />
6566
<Compile Include="System\Net\WebSockets\HttpWebSocket.cs" />
@@ -87,6 +88,9 @@
8788
<Compile Include="$(CommonPath)\System\Net\HttpKnownHeaderNames.cs">
8889
<Link>Common\System\Net\HttpKnownHeaderNames.cs</Link>
8990
</Compile>
91+
<Compile Include="$(CommonPath)\System\Net\LazyAsyncResult.cs">
92+
<Link>Common\System\Net\LazyAsyncResult.cs</Link>
93+
</Compile>
9094
<Compile Include="$(CommonPath)\System\Net\UriScheme.cs">
9195
<Link>Common\System\Net\UriScheme.cs</Link>
9296
</Compile>
@@ -108,7 +112,6 @@
108112
<Compile Include="System\Net\Windows\HttpListenerRequestContext.cs" />
109113
<Compile Include="System\Net\Windows\ListenerClientCertAsyncResult.Windows.cs" />
110114
<Compile Include="System\Net\Windows\AsyncRequestContext.cs" />
111-
<Compile Include="System\Net\Windows\ListenerClientCertState.cs" />
112115
<Compile Include="System\Net\Windows\EntitySendFormat.cs" />
113116
<Compile Include="System\Net\Windows\RequestContextBase.cs" />
114117
<Compile Include="System\Net\Windows\SyncRequestContext.cs" />
@@ -122,9 +125,6 @@
122125
<Compile Include="$(CommonPath)\System\Net\InternalException.cs">
123126
<Link>Common\System\Net\InternalException.cs</Link>
124127
</Compile>
125-
<Compile Include="$(CommonPath)\System\Net\LazyAsyncResult.cs">
126-
<Link>Common\System\Net\LazyAsyncResult.cs</Link>
127-
</Compile>
128128
<Compile Include="$(CommonPath)\System\Net\WebHeaderEncoding.cs">
129129
<Link>Common\System\Net\WebHeaderEncoding.cs</Link>
130130
</Compile>

‎src/System.Net.HttpListener/src/System/Net/HttpListenerRequest.cs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
using System.Globalization;
99
using System.Net.WebSockets;
1010
using System.Reflection;
11+
using System.Security.Cryptography.X509Certificates;
1112
using System.Text;
13+
using System.Threading.Tasks;
1214

1315
namespace System.Net
1416
{
@@ -269,6 +271,61 @@ public Uri UrlReferrer
269271
public Uri Url => RequestUri;
270272

271273
public Version ProtocolVersion => _version;
274+
275+
public X509Certificate2 GetClientCertificate()
276+
{
277+
if (NetEventSource.IsEnabled) NetEventSource.Enter(this);
278+
try
279+
{
280+
if (ClientCertState == ListenerClientCertState.InProgress)
281+
throw new InvalidOperationException(SR.Format(SR.net_listener_callinprogress, $"{nameof(GetClientCertificate)}()/{nameof(BeginGetClientCertificate)}()"));
282+
ClientCertState = ListenerClientCertState.InProgress;
283+
284+
GetClientCertificateCore();
285+
286+
ClientCertState = ListenerClientCertState.Completed;
287+
if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"_clientCertificate:{ClientCertificate}");
288+
}
289+
finally
290+
{
291+
if (NetEventSource.IsEnabled) NetEventSource.Exit(this);
292+
}
293+
return ClientCertificate;
294+
}
295+
296+
public IAsyncResult BeginGetClientCertificate(AsyncCallback requestCallback, object state)
297+
{
298+
if (NetEventSource.IsEnabled) NetEventSource.Info(this);
299+
if (ClientCertState == ListenerClientCertState.InProgress)
300+
throw new InvalidOperationException(SR.Format(SR.net_listener_callinprogress, $"{nameof(GetClientCertificate)}()/{nameof(BeginGetClientCertificate)}()"));
301+
ClientCertState = ListenerClientCertState.InProgress;
302+
303+
return BeginGetClientCertificateCore(requestCallback, state);
304+
}
305+
306+
public Task<X509Certificate2> GetClientCertificateAsync()
307+
{
308+
return Task.Factory.FromAsync(
309+
(callback, state) => ((HttpListenerRequest)state).BeginGetClientCertificate(callback, state),
310+
iar => ((HttpListenerRequest)iar.AsyncState).EndGetClientCertificate(iar),
311+
this);
312+
}
313+
314+
internal ListenerClientCertState ClientCertState { get; set; } = ListenerClientCertState.NotInitialized;
315+
internal X509Certificate2 ClientCertificate { get; set; }
316+
317+
public int ClientCertificateError
318+
{
319+
get
320+
{
321+
if (ClientCertState == ListenerClientCertState.NotInitialized)
322+
throw new InvalidOperationException(SR.Format(SR.net_listener_mustcall, "GetClientCertificate()/BeginGetClientCertificate()"));
323+
else if (ClientCertState == ListenerClientCertState.InProgress)
324+
throw new InvalidOperationException(SR.Format(SR.net_listener_mustcompletecall, "GetClientCertificate()/BeginGetClientCertificate()"));
325+
326+
return GetClientCertificateErrorCore();
327+
}
328+
}
272329

273330
private static class Helpers
274331
{

src/System.Net.HttpListener/src/System/Net/Windows/ListenerClientCertState.cs renamed to src/System.Net.HttpListener/src/System/Net/ListenerClientCertState.cs

File renamed without changes.

‎src/System.Net.HttpListener/src/System/Net/Managed/HttpListenerRequest.Managed.cs‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,9 @@
3333
using System.Collections.Specialized;
3434
using System.Globalization;
3535
using System.IO;
36-
using System.Net.WebSockets;
3736
using System.Security.Authentication.ExtendedProtection;
3837
using System.Security.Cryptography.X509Certificates;
3938
using System.Text;
40-
using System.Threading.Tasks;
4139

4240
namespace System.Net
4341
{
@@ -294,18 +292,17 @@ internal bool FlushInput()
294292
}
295293
}
296294

297-
public int ClientCertificateError
295+
private X509Certificate2 GetClientCertificateCore() => ClientCertificate = _context.Connection.ClientCertificate;
296+
297+
private int GetClientCertificateErrorCore()
298298
{
299-
get
300-
{
301-
HttpConnection cnc = _context.Connection;
302-
if (cnc.ClientCertificate == null)
303-
return 0;
304-
int[] errors = cnc.ClientCertificateErrors;
305-
if (errors != null && errors.Length > 0)
306-
return errors[0];
299+
HttpConnection cnc = _context.Connection;
300+
if (cnc.ClientCertificate == null)
307301
return 0;
308-
}
302+
int[] errors = cnc.ClientCertificateErrors;
303+
if (errors != null && errors.Length > 0)
304+
return errors[0];
305+
return 0;
309306
}
310307

311308
public long ContentLength64
@@ -351,32 +348,47 @@ public Stream InputStream
351348

352349
public Guid RequestTraceIdentifier => Guid.Empty;
353350

354-
public IAsyncResult BeginGetClientCertificate(AsyncCallback requestCallback, object state)
351+
private IAsyncResult BeginGetClientCertificateCore(AsyncCallback requestCallback, object state)
355352
{
356-
Task<X509Certificate2> getClientCertificate = new Task<X509Certificate2>(() => GetClientCertificate());
357-
return TaskToApm.Begin(getClientCertificate, requestCallback, state);
353+
var asyncResult = new GetClientCertificateAsyncResult(this, state, requestCallback);
354+
355+
// The certificate is already retrieved by the time this method is called. GetClientCertificateCore() evaluates to
356+
// a simple member access, so this will always complete immediately.
357+
ClientCertState = ListenerClientCertState.Completed;
358+
asyncResult.InvokeCallback(GetClientCertificateCore());
359+
360+
return asyncResult;
358361
}
359362

360363
public X509Certificate2 EndGetClientCertificate(IAsyncResult asyncResult)
361364
{
362365
if (asyncResult == null)
363366
throw new ArgumentNullException(nameof(asyncResult));
364-
365-
return TaskToApm.End<X509Certificate2>(asyncResult);
366-
}
367367

368-
public X509Certificate2 GetClientCertificate() => _context.Connection.ClientCertificate;
368+
GetClientCertificateAsyncResult clientCertAsyncResult = asyncResult as GetClientCertificateAsyncResult;
369+
if (clientCertAsyncResult == null || clientCertAsyncResult.AsyncObject != this)
370+
{
371+
throw new ArgumentException(SR.net_io_invalidasyncresult, nameof(asyncResult));
372+
}
373+
if (clientCertAsyncResult.EndCalled)
374+
{
375+
throw new InvalidOperationException(SR.Format(SR.net_io_invalidendcall, nameof(EndGetClientCertificate)));
376+
}
377+
clientCertAsyncResult.EndCalled = true;
378+
379+
return (X509Certificate2)clientCertAsyncResult.Result;
380+
}
369381

370382
public string ServiceName => null;
371383

372384
public TransportContext TransportContext => new Context();
373385

374-
public Task<X509Certificate2> GetClientCertificateAsync()
375-
{
376-
return Task<X509Certificate2>.Factory.FromAsync(BeginGetClientCertificate, EndGetClientCertificate, null);
377-
}
378-
379386
private Uri RequestUri => _requestUri;
380387
private bool SupportsWebSockets => true;
388+
389+
private class GetClientCertificateAsyncResult : LazyAsyncResult
390+
{
391+
public GetClientCertificateAsyncResult(object myObject, object myState, AsyncCallback myCallBack) : base(myObject, myState, myCallBack) { }
392+
}
381393
}
382394
}

‎src/System.Net.HttpListener/src/System/Net/Windows/HttpListenerRequest.Windows.cs‎

Lines changed: 7 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
using System.Security.Cryptography.X509Certificates;
1515
using System.Security.Principal;
1616
using System.Text;
17-
using System.Threading.Tasks;
1817

1918
namespace System.Net
2019
{
@@ -33,8 +32,6 @@ public sealed unsafe partial class HttpListenerRequest
3332
private IPEndPoint _localEndPoint;
3433
private IPEndPoint _remoteEndPoint;
3534
private BoundaryType _boundaryType;
36-
private ListenerClientCertState _clientCertState;
37-
private X509Certificate2 _clientCertificate;
3835
private int _clientCertificateError;
3936
private RequestContextBase _memoryBlob;
4037
private HttpListenerContext _httpContext;
@@ -86,7 +83,6 @@ internal HttpListenerRequest(HttpListenerContext httpContext, RequestContextBase
8683
_cookedUrlQuery = Marshal.PtrToStringUni((IntPtr)cookedUrl.pQueryString, cookedUrl.QueryStringLength / 2);
8784
}
8885
_version = new Version(memoryBlob.RequestBlob->Version.MajorVersion, memoryBlob.RequestBlob->Version.MinorVersion);
89-
_clientCertState = ListenerClientCertState.NotInitialized;
9086
if (NetEventSource.IsEnabled)
9187
{
9288
NetEventSource.Info(this, $"RequestId:{RequestId} ConnectionId:{_connectionId} RawConnectionId:{memoryBlob.RequestBlob->RawConnectionId} UrlContext:{memoryBlob.RequestBlob->UrlContext} RawUrl:{_rawUrl} Version:{_version} Secure:{_sslStatus}");
@@ -249,56 +245,17 @@ public string ServiceName
249245
internal set => _serviceName = value;
250246
}
251247

252-
public int ClientCertificateError
248+
private int GetClientCertificateErrorCore()
253249
{
254-
get
255-
{
256-
if (_clientCertState == ListenerClientCertState.NotInitialized)
257-
throw new InvalidOperationException(SR.Format(SR.net_listener_mustcall, "GetClientCertificate()/BeginGetClientCertificate()"));
258-
else if (_clientCertState == ListenerClientCertState.InProgress)
259-
throw new InvalidOperationException(SR.Format(SR.net_listener_mustcompletecall, "GetClientCertificate()/BeginGetClientCertificate()"));
260-
261-
if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"ClientCertificateError:{_clientCertificateError}");
262-
return _clientCertificateError;
263-
}
264-
}
265-
266-
internal X509Certificate2 ClientCertificate
267-
{
268-
set => _clientCertificate = value;
269-
}
270-
271-
internal ListenerClientCertState ClientCertState
272-
{
273-
set => _clientCertState = value;
250+
if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"ClientCertificateError:{_clientCertificateError}");
251+
return _clientCertificateError;
274252
}
275253

276254
internal void SetClientCertificateError(int clientCertificateError)
277255
{
278256
_clientCertificateError = clientCertificateError;
279257
}
280258

281-
public X509Certificate2 GetClientCertificate()
282-
{
283-
if (NetEventSource.IsEnabled) NetEventSource.Enter(this);
284-
try
285-
{
286-
ProcessClientCertificate();
287-
if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"_clientCertificate:{_clientCertificate}");
288-
}
289-
finally
290-
{
291-
if (NetEventSource.IsEnabled) NetEventSource.Exit(this);
292-
}
293-
return _clientCertificate;
294-
}
295-
296-
public IAsyncResult BeginGetClientCertificate(AsyncCallback requestCallback, object state)
297-
{
298-
if (NetEventSource.IsEnabled) NetEventSource.Info(this);
299-
return AsyncProcessClientCertificate(requestCallback, state);
300-
}
301-
302259
public X509Certificate2 EndGetClientCertificate(IAsyncResult asyncResult)
303260
{
304261
if (NetEventSource.IsEnabled) NetEventSource.Enter(this);
@@ -320,7 +277,7 @@ public X509Certificate2 EndGetClientCertificate(IAsyncResult asyncResult)
320277
}
321278
clientCertAsyncResult.EndCalled = true;
322279
clientCertificate = clientCertAsyncResult.InternalWaitForCompletion() as X509Certificate2;
323-
if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"_clientCertificate:{_clientCertificate}");
280+
if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"_clientCertificate:{ClientCertificate}");
324281
}
325282
finally
326283
{
@@ -329,14 +286,6 @@ public X509Certificate2 EndGetClientCertificate(IAsyncResult asyncResult)
329286
return clientCertificate;
330287
}
331288

332-
public Task<X509Certificate2> GetClientCertificateAsync()
333-
{
334-
return Task.Factory.FromAsync(
335-
(callback, state) => ((HttpListenerRequest)state).BeginGetClientCertificate(callback, state),
336-
iar => ((HttpListenerRequest)iar.AsyncState).EndGetClientCertificate(iar),
337-
this);
338-
}
339-
340289
public TransportContext TransportContext => new HttpListenerRequestContext(this);
341290

342291
public bool HasEntityBody
@@ -389,12 +338,8 @@ internal void Close()
389338
if (NetEventSource.IsEnabled) NetEventSource.Exit(this);
390339
}
391340

392-
private ListenerClientCertAsyncResult AsyncProcessClientCertificate(AsyncCallback requestCallback, object state)
341+
private ListenerClientCertAsyncResult BeginGetClientCertificateCore(AsyncCallback requestCallback, object state)
393342
{
394-
if (_clientCertState == ListenerClientCertState.InProgress)
395-
throw new InvalidOperationException(SR.Format(SR.net_listener_callinprogress, "GetClientCertificate()/BeginGetClientCertificate()"));
396-
_clientCertState = ListenerClientCertState.InProgress;
397-
398343
ListenerClientCertAsyncResult asyncResult = null;
399344
//--------------------------------------------------------------------
400345
//When you configure the HTTP.SYS with a flag value 2
@@ -490,11 +435,8 @@ private ListenerClientCertAsyncResult AsyncProcessClientCertificate(AsyncCallbac
490435
return asyncResult;
491436
}
492437

493-
private void ProcessClientCertificate()
438+
private void GetClientCertificateCore()
494439
{
495-
if (_clientCertState == ListenerClientCertState.InProgress)
496-
throw new InvalidOperationException(SR.Format(SR.net_listener_callinprogress, "GetClientCertificate()/BeginGetClientCertificate()"));
497-
_clientCertState = ListenerClientCertState.InProgress;
498440
if (NetEventSource.IsEnabled) NetEventSource.Info(this);
499441
//--------------------------------------------------------------------
500442
//When you configure the HTTP.SYS with a flag value 2
@@ -572,7 +514,7 @@ private void ProcessClientCertificate()
572514
{
573515
byte[] certEncoded = new byte[pClientCertInfo->CertEncodedSize];
574516
Marshal.Copy((IntPtr)pClientCertInfo->pCertEncoded, certEncoded, 0, certEncoded.Length);
575-
_clientCertificate = new X509Certificate2(certEncoded);
517+
ClientCertificate = new X509Certificate2(certEncoded);
576518
}
577519
catch (CryptographicException exception)
578520
{
@@ -595,7 +537,6 @@ private void ProcessClientCertificate()
595537
break;
596538
}
597539
}
598-
_clientCertState = ListenerClientCertState.Completed;
599540
}
600541

601542
private Uri RequestUri

‎src/System.Net.HttpListener/tests/HttpListenerRequestTests.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ public async Task UserLanguages_GetProperty_ReturnsExpected(string userLanguageS
226226
}
227227

228228
[ConditionalFact(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotOneCoreUAP))]
229-
[PlatformSpecific(TestPlatforms.Windows)] // We get the ClientCertificate during connection on Unix.
230229
public async Task ClientCertificateError_GetNotInitialized_ThrowsInvalidOperationException()
231230
{
232231
await GetRequest("POST", null, null, (_, request) =>
@@ -259,7 +258,6 @@ await GetRequest("POST", null, null, (_, request) =>
259258
}
260259

261260
[ConditionalFact(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotOneCoreUAP))]
262-
[ActiveIssue(18128, TestPlatforms.AnyUnix)] // Hangs forever.
263261
public async Task GetClientCertificateAsync_NoCertificate_ReturnsNull()
264262
{
265263
await GetRequest("POST", null, null, (_, request) =>
@@ -278,7 +276,6 @@ await GetRequest("POST", null, null, (_, request) =>
278276
}
279277

280278
[ConditionalFact(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotOneCoreUAP))]
281-
[PlatformSpecific(TestPlatforms.Windows)] // We get the ClientCertificate during connection on Unix.
282279
public async Task EndGetClientCertificate_InvalidAsyncResult_ThrowsArgumentException()
283280
{
284281
await GetRequest("POST", null, null, (socket1, request1) =>
@@ -294,7 +291,6 @@ await GetRequest("POST", null, null, (socket1, request1) =>
294291
}
295292

296293
[ConditionalFact(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotOneCoreUAP))]
297-
[PlatformSpecific(TestPlatforms.Windows)] // We get the ClientCertificate during connection on Unix.
298294
public async Task EndGetClientCertificate_AlreadyCalled_ThrowsInvalidOperationException()
299295
{
300296
await GetRequest("POST", null, null, (_, request) =>

0 commit comments

Comments
 (0)