[.NET 10] Runtime issue when adding a `MaxLengthAttribute` to a Guid: "The field of type System.Guid must be a string..."

[erwinkramer]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

I'm setting up validation but it doesn't seem to accept a MaxLengthAttribute construction when the type is a Guid.

Now you might ask, why does it make sense to add a maxLength validation to a Guid, when it's already a uuid in the OpenAPI definition, and that can only be 36-characters? Well, it's mainly because this spectral rule requests it: https://github.com/stoplightio/spectral-owasp-ruleset/blob/624d67f3a124de36502ad3d06c35917da19dff85/src/ruleset.ts#L533. Really that might not be the best excuse, but it is one. A related and interesting discussion regarding length constraints and formats can be found here: stoplightio/spectral-owasp-ruleset#41. Per OpenAPI spec (https://spec.openapis.org/oas/v3.1.1.html#data-type-format): "Tools that do not recognize a specific format MAY default back to the type alone, as if the format is not specified", so that might be another excuse to just have the maxLength in.

The validation message particularly seems strange. It might be better to give a build-time error, since it's just a broken setup that cannot accept any requests.

Expected Behavior

No response

Steps To Reproduce

Add this attribute to a Guid type:

public class IdAttribute : MaxLengthAttribute
{
    public IdAttribute() : base(36) { }
}

Response:

{
  "type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "instance": "POST /banks",
  "errors": {
    "Id": [
      "The field of type System.Guid must be a string, array or ICollection type."
    ]
  },
  "traceId": "00-c1e85619f97b5c95a37bd8a1aada4380-afe11599ea5775a9-01",
  "requestId": "0HNE4LBCI97OJ:0000000D"
}

Exceptions (if any)

No response

.NET Version

10.0.100-preview.6.25358.103

Anything else?

No response

[captainsafia]

@erwinkramer This particular issue arises because the [MaxLength] attribute attempts to validate that the incoming type has a Count property before it evaluates its length.

https://github.com/dotnet/runtime/blob/23323e91728ba356ea8ed029631a63a55d90907a/src/libraries/System.ComponentModel.Annotations/src/System/ComponentModel/DataAnnotations/MaxLengthAttribute.cs#L82-L85

There are a couple of ways to resolve it:

• Update the implementation of the attributes in the BCL to tolerate more types. It would be worth filing an issue on dotnet/runtime for this.
• Declare a custom validation attribute for IdAttribute instead of inserting from MaxLengthAttribute although that would be you'd have to customize the OpenAPI schema yourself to reflect that it has a maxLength.

[erwinkramer]

I filed an issue on dotnet/runtime for this.

For now, I just customized the OpenAPI schema, which works fine: erwinkramer/bank-api@42f5cfc

For another possible way to resolve: can the mapper (https://learn.microsoft.com/en-us/aspnet/core/fundamentals/openapi/include-metadata?view=aspnetcore-9.0&tabs=minimal-apis#type-and-format) generate an (optional, but predefined) maxLength when it's a Guid? Same could then be done for other predictable maximums for types, such as DateTimeOffset to 33.

[captainsafia]

@mikekistler What are your thoughts on this? Should we be adding additional info around maxLength for things like Guid and DateTimeOffset?

For another possible way to resolve: can the mapper (learn.microsoft.com/en-us/aspnet/core/fundamentals/openapi/include-metadata?view=aspnetcore-9.0&tabs=minimal-apis#type-and-format) generate an (optional, but predefined) maxLength when it's a Guid? Same could then be done for other predictable maximums for types, such as DateTimeOffset to 33.

[mikekistler]

I'm not keen on adding a maxLength in these cases. If the value should be a GUID, the check should be that the value is a GUID -- not that and that it has the proper length. If we added maxLength, would we also have to add minLength, since that can also be inferred. And there is also a pattern we could add -- only 0-9a-f and -. But all of these would be redundant with validating that the value is indeed a GUID. People already complain about how bulky OpenAPI documents are. We should avoid adding redundant information wherever possible.

[captainsafia]

Yeah, I'm inclined to say that for these kinds of things someone should author a SpectralSchemaTransformer that applies a set of changes to comply with Spectral's ruleset. I don't think we should optimize for a certain opinion in the default implementation, especially if it involves encoding more information than needed.

[erwinkramer]

Thanks for discussing all options, final proposal makes sense for me too.