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

Commit aab344a

Browse files
committed
apply review feedback
1 parent b3b5f59 commit aab344a

File tree

5 files changed

+41
-24
lines changed

5 files changed

+41
-24
lines changed

src/Common/tests/System/Net/VirtualNetwork/VirtualNetwork.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ public VirtualNetworkConnectionBroken() : base("Connection broken") { }
2828
public void ReadFrame(bool server, out byte[] buffer)
2929
{
3030
if (_connectionBroken)
31+
{
3132
throw new VirtualNetworkConnectionBroken();
33+
}
3234

3335
SemaphoreSlim semaphore;
3436
ConcurrentQueue<byte[]> packetQueue;
@@ -50,7 +52,9 @@ public void ReadFrame(bool server, out byte[] buffer)
5052
}
5153

5254
if (_connectionBroken)
55+
{
5356
throw new VirtualNetworkConnectionBroken();
57+
}
5458

5559
bool dequeueSucceeded = false;
5660
int remainingTries = 3;
@@ -76,7 +80,9 @@ public void ReadFrame(bool server, out byte[] buffer)
7680
public void WriteFrame(bool server, byte[] buffer)
7781
{
7882
if (_connectionBroken)
83+
{
7984
throw new VirtualNetworkConnectionBroken();
85+
}
8086

8187
SemaphoreSlim semaphore;
8288
ConcurrentQueue<byte[]> packetQueue;
@@ -102,8 +108,8 @@ public void WriteFrame(bool server, byte[] buffer)
102108
public void BreakConnection()
103109
{
104110
_connectionBroken = true;
105-
_serverDataAvailable.Release(1000000);
106-
_clientDataAvailable.Release(1000000);
111+
_serverDataAvailable.Release(1_000_000);
112+
_clientDataAvailable.Release(1_000_000);
107113
}
108114
}
109115
}

src/System.Net.Security/ref/System.Net.Security.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public enum EncryptionPolicy
3232
RequireEncryption = 0,
3333
}
3434
public delegate System.Security.Cryptography.X509Certificates.X509Certificate LocalCertificateSelectionCallback(object sender, string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection localCertificates, System.Security.Cryptography.X509Certificates.X509Certificate remoteCertificate, string[] acceptableIssuers);
35-
public delegate X509Certificate ServerCertificateSelectionCallback(object sender, string hostName);
35+
public delegate System.Security.Cryptography.X509Certificates.X509Certificate ServerCertificateSelectionCallback(object sender, string hostName);
3636

3737
public partial class NegotiateStream : AuthenticatedStream
3838
{
@@ -182,7 +182,6 @@ public virtual void AuthenticateAsClient(string targetHost, System.Security.Cryp
182182
public virtual void AuthenticateAsServer(System.Security.Cryptography.X509Certificates.X509Certificate serverCertificate) { }
183183
public virtual void AuthenticateAsServer(System.Security.Cryptography.X509Certificates.X509Certificate serverCertificate, bool clientCertificateRequired, System.Security.Authentication.SslProtocols enabledSslProtocols, bool checkCertificateRevocation) { }
184184
public virtual void AuthenticateAsServer(System.Security.Cryptography.X509Certificates.X509Certificate serverCertificate, bool clientCertificateRequired, bool checkCertificateRevocation) { }
185-
public void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthenticationOptions) { }
186185
public virtual System.Threading.Tasks.Task AuthenticateAsClientAsync(string targetHost) { throw null; }
187186
public virtual System.Threading.Tasks.Task AuthenticateAsClientAsync(string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection clientCertificates, System.Security.Authentication.SslProtocols enabledSslProtocols, bool checkCertificateRevocation) { throw null; }
188187
public virtual System.Threading.Tasks.Task AuthenticateAsClientAsync(string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection clientCertificates, bool checkCertificateRevocation) { throw null; }

src/System.Net.Security/src/System/Net/Security/SslStream.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,16 +351,14 @@ public virtual void AuthenticateAsServer(X509Certificate serverCertificate, bool
351351
AuthenticateAsServer(options);
352352
}
353353

354-
public void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthenticationOptions)
354+
private void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthenticationOptions)
355355
{
356356
SecurityProtocol.ThrowOnNotAllowed(sslServerAuthenticationOptions.EnabledSslProtocols);
357357
SetAndVerifyValidationCallback(sslServerAuthenticationOptions.RemoteCertificateValidationCallback);
358358

359359
// Set the delegate on the options.
360360
sslServerAuthenticationOptions._certValidationDelegate = _certValidationDelegate;
361361

362-
SetServerCertificateSelectionCallbackWrapper(sslServerAuthenticationOptions);
363-
364362
_sslState.ValidateCreateContext(sslServerAuthenticationOptions);
365363
_sslState.ProcessAuthentication(null);
366364
}

src/System.Net.Security/tests/FunctionalTests/SslStreamSNITest.cs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
using System;
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
26
using System.Collections.Generic;
37
using System.IO;
48
using System.Linq;
@@ -7,6 +11,7 @@
711
using System.Security.Authentication;
812
using System.Security.Cryptography.X509Certificates;
913
using System.Text;
14+
using System.Threading;
1015
using System.Threading.Tasks;
1116
using Xunit;
1217

@@ -17,11 +22,16 @@ namespace System.Net.Security.Tests
1722
[Trait("feature", "sni")]
1823
public class SslStreamSNITest
1924
{
25+
private static IEnumerable<object[]> HostNameData()
26+
{
27+
yield return new object[] { "a" };
28+
yield return new object[] { "test" };
29+
yield return new object[] { new string('a', 100) };
30+
}
31+
2032
[Theory]
21-
[InlineData("a")]
22-
[InlineData("test")]
23-
[InlineData("aaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc")]
24-
public void ClientSendsSNIServerReceivesIt(string hostName)
33+
[MemberData(nameof(HostNameData))]
34+
public void SslStream_ClientSendsSNIServerReceives_Ok(string hostName)
2535
{
2636
X509Certificate serverCert = Configuration.Certificates.GetSelfSignedServerCertificate();
2737

@@ -33,17 +43,18 @@ public void ClientSendsSNIServerReceivesIt(string hostName)
3343

3444
SslServerAuthenticationOptions options = DefaultServerOptions();
3545

36-
bool callbackCalled = false;
46+
int timesCallbackCalled = 0;
3747
options.ServerCertificateSelectionCallback = (sender, actualHostName) =>
3848
{
39-
callbackCalled = true;
49+
timesCallbackCalled++;
4050
Assert.Equal(hostName, actualHostName);
4151
return serverCert;
4252
};
4353

44-
server.AuthenticateAsServer(options);
54+
var cts = new CancellationTokenSource();
55+
server.AuthenticateAsServerAsync(options, cts.Token).Wait();
4556

46-
Assert.True(callbackCalled);
57+
Assert.Equal(1, timesCallbackCalled);
4758
clientJob.Wait();
4859
},
4960
(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors) =>
@@ -54,7 +65,7 @@ public void ClientSendsSNIServerReceivesIt(string hostName)
5465
}
5566

5667
[Fact]
57-
public void ServerDoesNotKnowTheHostName()
68+
public void SslStream_UnknownHostName_Fails()
5869
{
5970
WithVirtualConnection((server, client) =>
6071
{
@@ -63,20 +74,23 @@ public void ServerDoesNotKnowTheHostName()
6374
=> client.AuthenticateAsClient("test"));
6475
});
6576

66-
bool callbackCalled = false;
77+
int timesCallbackCalled = 0;
6778
SslServerAuthenticationOptions options = DefaultServerOptions();
6879
options.ServerCertificateSelectionCallback = (sender, actualHostName) =>
6980
{
70-
callbackCalled = true;
81+
timesCallbackCalled++;
7182
return null;
7283
};
7384

74-
Assert.Throws<NotSupportedException>(() => {
75-
server.AuthenticateAsServer(options);
76-
});
85+
var cts = new CancellationTokenSource();
86+
Assert.ThrowsAsync<NotSupportedException>(async () => {
87+
await server.AuthenticateAsServerAsync(options, cts.Token);
88+
}).Wait();
89+
90+
// to break connection so that client is not waiting
7791
server.Dispose();
7892

79-
Assert.True(callbackCalled);
93+
Assert.Equal(1, timesCallbackCalled);
8094

8195
clientJob.Wait();
8296
},

src/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
<ItemGroup>
2323
<Compile Include="NotifyReadVirtualNetworkStream.cs" />
2424
<Compile Include="DummyTcpServer.cs" />
25-
<Compile Include="SslStreamSNITest.cs" />
2625
<Compile Include="TestConfiguration.cs" />
2726
<!-- SslStream Tests -->
2827
<Compile Include="CertificateChainValidation.cs" />
@@ -37,6 +36,7 @@
3736
<Compile Include="ServerRequireEncryptionTest.cs" />
3837
<Compile Include="SslStreamStreamToStreamTest.cs" />
3938
<Compile Include="SslStreamNetworkStreamTest.cs" />
39+
<Compile Include="SslStreamSNITest.cs" />
4040
<Compile Include="TransportContextTest.cs" />
4141
<!-- NegotiateStream Tests -->
4242
<Compile Include="NegotiateStreamStreamToStreamTest.cs" />

0 commit comments

Comments
 (0)