ArgumentNullException in ApplyContentTypeVersionActionFilter.ApplyApiVersionMediaTypeParameter(…)

[roxiemobile]

ApplyContentTypeVersionActionFilter throws an exception, if ApiVersioningOptions.ApiVersionReader property initialized with ApiVersionReader.Combine(…) method:

System.ArgumentNullException: Value cannot be null.
Parameter name: text
   at Microsoft.Extensions.Primitives.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at Microsoft.Extensions.Primitives.StringSegment.Equals(String text, StringComparison comparisonType)
   at Microsoft.AspNetCore.Mvc.Versioning.ApplyContentTypeVersionActionFilter.ApplyApiVersionMediaTypeParameter(Object state) in C:\BA\6915\s\src\Microsoft.AspNetCore.Mvc.Versioning\Versioning\ApplyContentTypeVersionActionFilter.cs:line 56
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.FireOnStartingMayAwait(Stack`1 onStarting)

Example of configuring API Versioning Options in our application:

public void ConfigureServices(IServiceCollection services) {
    …
    services.AddApiVersioning(
        options => {

            options.AssumeDefaultVersionWhenUnspecified = false;
            options.DefaultApiVersion = new ApiVersion(10, 0);
            options.ErrorResponses = new VendorErrorResponseProvider();

            options.ApiVersionReader = ApiVersionReader.Combine(
                new VendorMediaTypeApiVersionReader(new List<string> {"version", "v"}, options.DefaultApiVersion),
                new UrlSegmentApiVersionReader()
            );
        });
    …
}

Application starts crashing after migration from Microsoft.AspNetCore.Mvc.Versioning version 3.1.2 to >= 3.1.3. I'm investigate the problem and found that main reason of this crash is here: DescriptionContext.AddParameter(…)

[commonsensesoftware]

Interesting... in starting to dig into this, I still don't see where this would happen. I don't think it's necessarily specific to ApiVersionReader.Combine because that's almost always called. The default behavior combines QueryStringApiVersionReader and UrlSegmentApiVersionReader, which likely accounts for 80-90% of usage scenarios.

The change you are seeing was included in the last patch to address echoing back the API version in the response media type. I've looked at ApplyContentTypeVersionActionFilter and the call site you pointed out, but I'm not seeing how that would result in ArgumentNullException. The stack trace seems to indicate comparison against StringSegment, but I'm not seeing that used anywhere.

For completeness, can you share what you have for VendorMediaTypeApiVersionReader. I should be able to construct a repro with that. Versioning by both URL segment and media type does seem a bit odd, but it should work. It looks like you want to support multiple media type parameters. I suspect it may be related to that somehow. Thanks.

[roxiemobile]

«can you share what you have for VendorMediaTypeApiVersionReader» — Yeah, sure. I will create and upload working example of code, which leads to throwing this exception.

In short — it appears because of MediaTypeApiVersionReader sets values HasMediaTypeApiVersion = true and ParameterName = "v" and then UrlSegmentApiVersionReader sets value ParameterName = null.

After that reader.GetMediaTypeVersionParameter() returns null and application crashes on comparsion parameters[i].Name with parameterName in ApplyApiVersionMediaTypeParameter(…) because of parameterName = null.

[commonsensesoftware]

I probably don't need a full repro, just that class should do.

💡 Duh! Now I think I see what you're saying. I tried to get trickying with inlining, but that's wrong. The ParameterName should only be set on the pass where location == MediaTypeParameter. It's purely an ordering problem. Since the first pass marks it true, the second pass still enters the block even though it's false, which causes ParameterName to become null. 😣

This should be a simple fix. In the meantime, you should be able to unblock yourself by simply changing the order in which the IApiVersionReader instances are registered. This issue should not occur if UrlSegmentApiVersionReader is registered first. There is no short-circuiting logic. All readers are executed to account for a scenario where multiple API versions are allowed, specified, but are different (client error). Thanks.

[roxiemobile]

Yes, you right. Exactly this scenario leads to crash of application.
Will wait for fixed version of NuGet 😋👍🏼

[commonsensesoftware]

FYI ... the patch for this has been published.