Skip to content

Fix sanitization of type names in validations generator #61952

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

Merged
merged 1 commit into from
May 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@
using System.Text;
using Microsoft.CodeAnalysis.CSharp;
using System.IO;
using System.Text.RegularExpressions;

namespace Microsoft.AspNetCore.Http.ValidationsGenerator;

public sealed partial class ValidationsGenerator : IIncrementalGenerator
{
public static string GeneratedCodeConstructor => $@"global::System.CodeDom.Compiler.GeneratedCodeAttribute(""{typeof(ValidationsGenerator).Assembly.FullName}"", ""{typeof(ValidationsGenerator).Assembly.GetName().Version}"")";
public static string GeneratedCodeAttribute => $"[{GeneratedCodeConstructor}]";
private static readonly Regex InvalidNameCharsRegex = new("[^0-9A-Za-z_]", RegexOptions.Compiled);

internal static void Emit(SourceProductionContext context, (InterceptableLocation? AddValidation, ImmutableArray<ValidatableType> ValidatableTypes) emitInputs)
{
Expand Down Expand Up @@ -238,12 +240,7 @@ private static void EmitValidatableMemberForCreate(ValidatableProperty member, C

private static string SanitizeTypeName(string typeName)
{
// Replace invalid characters with underscores and remove generic notation
return typeName
.Replace(".", "_")
.Replace("<", "_")
.Replace(">", "_")
.Replace(",", "_")
.Replace(" ", "_");
// Replace invalid characters with underscores
return InvalidNameCharsRegex.Replace(typeName, "_");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't start with a digit presumably?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a restriction on metadata names starting with digits per the CLR spec. However, since we derive this MetadataName that we pass to this method from a TypeSymbol resolve via static analysis, it's probably come from C# code where an identifier starting with a number is invalid.

If we do hit that case, I think emitting a method name starting with _ is OK syntax so the generator shouldn't emit invalid syntax.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task CanValidateParameters()

var app = builder.Build();

app.MapGet("/params", (
app.MapPost("/params", (
// Skipped from validation because it is resolved as a service by IServiceProviderIsService
TestService testService,
// Skipped from validation because it is marked as a [FromKeyedService] parameter
Expand All @@ -39,7 +39,8 @@ public async Task CanValidateParameters()
[CustomValidation(ErrorMessage = "Value must be an even number")] int value4 = 4,
[CustomValidation, Range(10, 100)] int value5 = 10,
// Skipped from validation because it is marked as a [FromService] parameter
[FromServices] [Range(10, 100)] int? value6 = 4) => "OK");
[FromServices] [Range(10, 100)] int? value6 = 4,
Dictionary<string, TestService>? testDict = null) => "OK");

app.Run();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public bool TryGetValidatableTypeInfo(global::System.Type type, [global::System.
validatableInfo = CreateTestService();
return true;
}
if (type == typeof(global::System.Collections.Generic.Dictionary<string, global::TestService>))
{
validatableInfo = CreateDictionary_2();
return true;
}

return false;
}
Expand All @@ -92,6 +97,38 @@ private ValidatableTypeInfo CreateTestService()
]
);
}
private ValidatableTypeInfo CreateDictionary_2()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#61953 will change how this looks but we validate that compilable names are emitted here.

{
return new GeneratedValidatableTypeInfo(
type: typeof(global::System.Collections.Generic.Dictionary<string, global::TestService>),
members: [
new GeneratedValidatablePropertyInfo(
containingType: typeof(global::System.Collections.Generic.Dictionary<string, global::TestService>),
propertyType: typeof(global::System.Collections.Generic.ICollection<global::TestService>),
name: "System.Collections.Generic.IDictionary<TKey,TValue>.Values",
displayName: "System.Collections.Generic.IDictionary<TKey,TValue>.Values"
),
new GeneratedValidatablePropertyInfo(
containingType: typeof(global::System.Collections.Generic.Dictionary<string, global::TestService>),
propertyType: typeof(global::System.Collections.Generic.IEnumerable<global::TestService>),
name: "System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.Values",
displayName: "System.Collections.Generic.IReadOnlyDictionary<TKey,TValue>.Values"
),
new GeneratedValidatablePropertyInfo(
containingType: typeof(global::System.Collections.Generic.Dictionary<string, global::TestService>),
propertyType: typeof(global::TestService),
name: "this[]",
displayName: "this[]"
),
new GeneratedValidatablePropertyInfo(
containingType: typeof(global::System.Collections.Generic.Dictionary<string, global::TestService>),
propertyType: typeof(global::System.Collections.ICollection),
name: "System.Collections.IDictionary.Values",
displayName: "System.Collections.IDictionary.Values"
),
]
);
}

}

Expand Down
Loading