AdvertiseApiVersionsAttribute causes stackoverflow when enumarated

[marius-w-nilsen]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

Adding [AdvertiseApiVersions] attribute to either Controller or Method causes stack overflow exception when enumarted.

Because AdvertiseApiVersionsAttribute.GetHashCode() doesn't call base.GetHashCode() it causes a stackoverflow when enumerated. I.e when Swagger generates docs (Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateResponse)

AdvertiseApiVersionsAttribute

Expected Behavior

Adding [AdvertiseApiVersions] does not cause stack overflow when enumerated.

Steps To Reproduce

Add [AdvertiseApiVersions(3.0)] attribute to a controller V2/Controllers/OrdersController.cs in your example (AspNetCore/OpenApiExample)

Exceptions (if any)

If I completely misunderstand the reason for [AdvertiseApiVersions]

.NET Version

7.0.100

Anything else?

I was unable to get the FormatProviderTest to run locally, otherwise this would've been a PR 😢

[commonsensesoftware]

Confirmed. This is a 🐞 . I'm surprised this has never been caught by code analysis or a test.

I just discovered a different issue related to reference assembly package. I don't understand why it ever worked, but that's probably why the test is failing. The problem happens in Visual Studio, but not the CLI. You might try dotnet build or just dotnet test and see if it works.

I work on get a patch out. Thanks for reporting it.

[marius-w-nilsen]

Thanks!
Yep, tests run from CLI,
Noticed potential culture problems (:scream:) in the formatting tests though.
Expected status to be "Major = 1, Minor = 0, Iteration = 1.0", but "Major = 1, Minor = 0, Iteration = 1,0" differs near ",0" (index 35).

Console.WriteLine(string.Format(new CultureInfo("nb-no"), "{0:N1}", 1));  //1,0 <-- not expected in test
Console.WriteLine(string.Format(new CultureInfo("en-us"), "{0:N1}", 1));  //1.0
Console.WriteLine(string.Format( CultureInfo.InvariantCulture, "{0:N1}", 1)); //1.0

From MultipleFormatParameterData

  yield return new object[] { provider, "Major = {0:V}, Minor = {0:v}, Iteration = {1:N1}", 1, "Major = 1, Minor = 0, Iteration = 1.0" };

In Culture "nb-no" a comma is used as decimal seperator which doesn't follow Microsoft API versioning guidelines as far as I can tell, but I must admit I don't know what the "Iteration" argument is used for.

Looking around if I can spot why, but if it's an automatic for you, at least the information is shared 😄

[commonsensesoftware]

Whoah! That is a good find. I guess I never thought about that (yay en-us only 🤣). Supporting IFormatProvider is being compliant with the API design, but - honestly - the only option that makes sense is CultureInfo.InvariantCulture. I mean, an API version is not culture-specific. The , is the standard decimal character in Arabic numerals, which is what is used by the majority of the world. I know en-us and en-gb are a few of cultures that use Arabic numerals, but with . instead of ,.

Aside: I've always wondered how you can tell the mantissa component with ,. Is 100,000 the same as 100.000? Maybe it's supposed to be 100000 versus 100,000 == 100.000 🤔 .

This is a fixable problem. I'll update the test harness so that it explicitly sets the CultureInfo.CurrentCulture to en-us. The code won't know that's happening, but that will be provide deterministic results - that will work.

Thanks for taking the time to try and sharing the results.

[commonsensesoftware]

6.2.1 of Asp.Versioning.Abstractions has been published with the fix. The fix will be merged into the final 7.0 version too.

[marius-w-nilsen]

Using 6.2.1 of Abstractions fixes the problem.
About the culture, i had to [AssumeCulture] in one more test before i got all tests in .sln to run.
MMM generates aug and not expected Aug

I tried to create a PR for this, excuse my novice OSS contribution knowledge: #936