Skip to content

HttpMethodAttribute: fix documentation and parameter check #11133

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
Jun 12, 2019

Conversation

Thiez
Copy link
Contributor

@Thiez Thiez commented Jun 12, 2019

The second constructor of the HttpMethodAttribute class claims that the template parameter may not be null, and no such claim is present about the httpMethods parameter. However, when httpMethods is null, an exception is thrown, while template is allowed to be null (as can be seen in the HttpMethodAttribute(IEnumerable<string>) constructor, which passes null.

In addition, the HttpMethodAttribute(IEnumerable<string>) constructor contains a null-check for the httpMethods parameter, but since this check is also in the HttpMethodAttribute(IEnumerable<string>, string) constructor this check is dead code.

The second constructor of the `HttpMethodAttribute` class claims that the `template` parameter may not be `null`, and no such claim is present about the `httpMethods` parameter. However, when `httpMethods` is `null`, an exception is thrown, while `template` *is* allowed to be null (as can be seen in the `HttpMethodAttribute(IEnumerable<string>)` constructor, which passes `null`.

In addition, the `HttpMethodAttribute(IEnumerable<string>)` constructor contains a `null`-check for the `httpMethods` parameter, but since this check is *also* in the `HttpMethodAttribute(IEnumerable<string>, string)` constructor this check is dead code.
@analogrelay analogrelay added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 12, 2019
@pranavkm pranavkm merged commit db72ea0 into dotnet:master Jun 12, 2019
@pranavkm
Copy link
Contributor

Thanks for the PR.

@Thiez
Copy link
Contributor Author

Thiez commented Jun 12, 2019

My pleasure :-)

@Thiez Thiez deleted the patch-1 branch June 12, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants