-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Add a test confirming that X509Certificate is trimmed #48295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
65d593d
5fc85a2
7b3f2ce
279f596
338644e
7fd977c
9b1cf95
7d68cae
20c245a
707b683
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<ItemGroup> | ||
<TestConsoleAppSourceFiles Include="SlimBuilderDoesNotDependOnX509Test.cs"> | ||
<DisabledFeatureSwitches>System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault</DisabledFeatureSwitches> | ||
<AdditionalSourceFiles>X509Utilities.cs</AdditionalSourceFiles> | ||
</TestConsoleAppSourceFiles> | ||
<TestConsoleAppSourceFiles Include="UseHttpsDoesDependOnX509Test.cs"> | ||
<DisabledFeatureSwitches>System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault</DisabledFeatureSwitches> | ||
<AdditionalSourceFiles>X509Utilities.cs</AdditionalSourceFiles> | ||
</TestConsoleAppSourceFiles> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using Microsoft.AspNetCore.Builder; | ||
amcasey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
var builder = WebApplication.CreateSlimBuilder(args); | ||
var app = builder.Build(); | ||
|
||
if (X509Utilities.HasCertificateType) | ||
{ | ||
return -1; | ||
} | ||
|
||
return 100; // Success |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Security.Cryptography.X509Certificates; | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.AspNetCore.Hosting; | ||
|
||
var builder = WebApplication.CreateSlimBuilder(args); | ||
|
||
builder.WebHost.UseKestrel(serverOptions => | ||
{ | ||
serverOptions.ListenLocalhost(5000, listenOptions => | ||
{ | ||
listenOptions.UseHttps(); | ||
}); | ||
}); | ||
|
||
try | ||
{ | ||
_ = builder.Build(); | ||
} | ||
catch (InvalidOperationException) | ||
{ | ||
// Expected if there's no dev cert installed | ||
} | ||
|
||
if (!X509Utilities.HasCertificateType) | ||
{ | ||
return -1; | ||
} | ||
|
||
return 100; // Success |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System; | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.Linq; | ||
using System.Reflection; | ||
using Microsoft.AspNetCore.Builder; | ||
|
||
#nullable enable | ||
|
||
public static class X509Utilities | ||
{ | ||
public static bool HasCertificateType | ||
{ | ||
get | ||
{ | ||
var certificateType = GetType("System.Security.Cryptography", "System.Security.Cryptography.X509Certificates.X509Certificate"); | ||
|
||
// We're checking for members, rather than just the presence of the type, | ||
// because Debugger Display types may reference it without actually | ||
// causing a meaningful binary size increase. | ||
return certificateType is not null && GetMembers(certificateType).Any(); | ||
Comment on lines
+20
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another approach we could take here is to disable the feature switch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about that but worried it would hide problems. I could add a separate test for that flag, if you think it's worthwhile. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What kind of problems? This mode would more closely align with the Native AOT behavior. So it seems the more preferrable to me. The fact that we are rooting types for Debugging published apps is less desirable to verify (IMO). |
||
} | ||
} | ||
|
||
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2057:UnrecognizedReflectionPattern", | ||
Justification = "Returning null when the type is unreferenced is desirable")] | ||
private static Type? GetType(string assemblyName, string typeName) | ||
{ | ||
return Type.GetType($"{typeName}, {assemblyName}"); | ||
} | ||
|
||
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", | ||
Justification = "Returning null when the type is unreferenced is desirable")] | ||
private static MemberInfo[] GetMembers(Type type) | ||
{ | ||
return type.GetMembers(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance | BindingFlags.DeclaredOnly); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.