Skip to content

Add support for options in form-binding and anti-forgery #49935

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 2 commits into from
Aug 9, 2023

Conversation

captainsafia
Copy link
Member

Closes #49844.

Contributes to #49543.

@ghost ghost added the area-runtime label Aug 8, 2023
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-runtime labels Aug 8, 2023
var formFeature = features.Get<IFormFeature>();

// Request form has not been read yet, so set the limits
if (formFeature == null || formFeature is { Form: null })
Copy link
Member

@Tratcher Tratcher Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember how you made FormFeature.Form throw if the anti-forgery token was bad? Is there any way someone could make that run before routing and then have this code throw?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FormFeature.Form will only throw if it detects IAntiforgeryValidationFeature has been set on the HttpContext. This can happen if either:

  • The anti-forgery middleware has ran and set it
  • The user has manually set it themselves

For the first case, the anti-forgery middleware is endpoint aware so running it before routing has configured the endpoint will not set the feature.

The second case is the more likely avenue that the user might arrive at this scenario, wherein they've configured their own middleware that sets the IAntiforgeryValidationFeature or they're setting endpoints manually with their own middleware then calling into our own anti-forgery middleware.

Under those circumstances, I'm fine throwing an exception for people here.

@captainsafia captainsafia merged commit 28e5590 into main Aug 9, 2023
@captainsafia captainsafia deleted the safia/formbinding branch August 9, 2023 01:18
@ghost ghost added this to the 8.0-rc1 milestone Aug 9, 2023
@sebastienros
Copy link
Member

This is regressing all benchmarks using endpoint routing. Plaintext regresses by 16% for instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add metadata types for form reading and mapping options
3 participants