Skip to content

Fix non-parameter route constraints not called with endpoint routing for 2.2 #6587

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
Jan 15, 2019

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Jan 11, 2019

Addresses #6544

Description

Route constraints that do not have a matching parameter in the pattern are not run when matching a request with endpoint routing.

In the route below the DomainMatchingConstraint will not be run when matching a request because domain is not a route parameter.

routes.MapRoute(
    "default",
    "{controller=Home}/{action=Index}/{id?}",
    constraints: new { domain = new DomainMatchingConstraint("example.com"), });

Customer Impact

Requests will get matched to unexpected endpoints, e.g. the wrong MVC action. The only easy workaround to the issue is for the customer to disable endpoint routing.

Regression?

This is a regression for customers who upgrade to ASP.NET Core 2.2 and set 2.2/latest compatibility in their Startup.cs.

Risk

The fix is a small change in how endpoints are built in MVC. Risk is incorrect logic or the introduction of a runtime error.

@JamesNK
Copy link
Member Author

JamesNK commented Jan 11, 2019

@natemcmaster I needed to add dependency projects to Mvc.sln. Should it not have them or has no one needed to add them until now?

@natemcmaster
Copy link
Contributor

Should it not have them or has no one needed to add them until now?

More likely the second one. If people are restoring on command line and then opening VS, they may not see this. Also, they might not see these errors anymore if using the latest VS 2019 dogfood builds, which should soon have a fix for NU1105.

@JamesNK JamesNK closed this Jan 11, 2019
@JamesNK JamesNK reopened this Jan 11, 2019
@JamesNK JamesNK closed this Jan 11, 2019
@JamesNK JamesNK reopened this Jan 11, 2019
@natemcmaster natemcmaster added this to the 2.2.x milestone Jan 11, 2019
@natemcmaster
Copy link
Contributor

Can you fill out the shiproom template and apply the servicing-consider label when its ready for shiproom to review?

@JamesNK JamesNK added the Servicing-consider Shiproom approval is required for the issue label Jan 13, 2019
@JamesNK JamesNK changed the title Fix non-parameter route constraints not called with endpoint routing Fix non-parameter route constraints not called with endpoint routing for 2.2 Jan 13, 2019
@JamesNK JamesNK force-pushed the jamesnk/nonparameterconstraint-fix branch from dd55f16 to 7801d8c Compare January 14, 2019 22:32
@JamesNK
Copy link
Member Author

JamesNK commented Jan 14, 2019

🆙 📅

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 15, 2019
@leecow leecow modified the milestones: 2.2.x, 2.2.2 Jan 15, 2019
@natemcmaster natemcmaster requested a review from dougbu January 15, 2019 19:32
@natemcmaster
Copy link
Contributor

Can you update eng/PatchConfig.props to list the packages which need to ship an update?

@dougbu
Copy link
Contributor

dougbu commented Jan 15, 2019

@JamesNK in addition, please resolve the conflicts in the solution

@JamesNK JamesNK force-pushed the jamesnk/nonparameterconstraint-fix branch from f2b96bf to 81486bb Compare January 15, 2019 21:02
@JamesNK JamesNK merged commit 180f735 into release/2.2 Jan 15, 2019
@natemcmaster natemcmaster deleted the jamesnk/nonparameterconstraint-fix branch January 18, 2019 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants