Skip to content
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

[Bug]: Wrong exception thrown by MemberInfoExtensions.IsDictionaryValueNonNullable #3070

Open
mywyb2 opened this issue Sep 12, 2024 · 4 comments · May be fixed by #3119
Open

[Bug]: Wrong exception thrown by MemberInfoExtensions.IsDictionaryValueNonNullable #3070

mywyb2 opened this issue Sep 12, 2024 · 4 comments · May be fixed by #3119
Assignees
Labels

Comments

@mywyb2
Copy link

mywyb2 commented Sep 12, 2024

Describe the bug

Starting from version 6.7.2, MemberInfoExtensions.IsDictionaryValueNonNullable contains a bug that throws an exception when the declaring type of a class property that is assignable to Dictionary{TKey,TValue} has a number of generic type arguments that is not 2. This is easily possible when the declaring type extends Dictionary{TKey,TValue}, e.g.

public class MyDictionary: Dictionary<string, string>

Instead of picking the generic type arguments of the declaring type, you need to look at the type arguments of the IDictionary-interface of the declaring type.

Expected behavior

That no exception is thrown, just like in versions <= 6.7.1.

Actual behavior

An exception is thrown on initialization, causing Swagger to fail to load API definition.

Steps to reproduce

No response

Exception(s) (if any)

No response

Swashbuckle.AspNetCore version

6.7.2

.NET Version

8

Anything else?

No response

@mywyb2 mywyb2 added the bug label Sep 12, 2024
@mywyb2 mywyb2 changed the title [Bug]: Wrong exception in 6.7.2 [Bug]: Wrong exception thrown by MemberInfoExtensions.IsDictionaryValueNonNullable Sep 12, 2024
@martincostello martincostello added the help-wanted A change up for grabs for contributions from the community label Sep 12, 2024
@martincostello
Copy link
Collaborator

For anyone who'd like to pick this up, the affected line of code is here:

if (genericArguments.Length != 2)
{
return false;
}

Instead, we need to recurse down through the type hierarchy until we get to the dictionary class, and then extract the type arguments from there. Otherwise there's no guarantee the first two type parameters will be the key and value, for example:

public class MyObtuseDictionary<TWhatever, TValue, TKey> : Dictionary<TKey, TValue>;

In this case, the key is generic argument [2], and the value is [1].

@mywyb2
Copy link
Author

mywyb2 commented Sep 15, 2024

The code that wrongly throws the exception is actually the block starting line 88.

@martincostello
Copy link
Collaborator

Thanks for clarifying.

Both pieces of code are wrong with respect to your scenario (deriving from a dictionary with more than 2 type parameters), it just depends on which TFM your app uses as to which variant is executed. One throws, the other returns the wrong result.

@s-kip
Copy link

s-kip commented Sep 30, 2024

I have the same problem with my custom dictionary implementation:

public class MyDictionary<T> : Dictionary<string, T>

Is there any kind of workaround or fix planned?

@martincostello martincostello removed the help-wanted A change up for grabs for contributions from the community label Oct 25, 2024
@martincostello martincostello self-assigned this Oct 25, 2024
martincostello added a commit to martincostello/Swashbuckle.AspNetCore that referenced this issue Oct 25, 2024
- Do not throw if we cannot determine the nullability of a dictionary.
- Clean-up some code analysis suggestions.

Resolves domaindrivendev#3070.
Resolves domaindrivendev#2793.
@martincostello martincostello linked a pull request Oct 25, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants