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

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

Merged
merged 2 commits into from Jan 15, 2019

Conversation

Projects
None yet
6 participants
@JamesNK
Copy link
Member

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 JamesNK requested review from rynowak and natemcmaster Jan 11, 2019

@JamesNK

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

natemcmaster commented Jan 11, 2019

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

This comment has been minimized.

Copy link
Member

natemcmaster commented Jan 11, 2019

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

@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 Jan 14, 2019

@JamesNK

This comment has been minimized.

Copy link
Member

JamesNK commented Jan 14, 2019

🆙 📅

@leecow leecow modified the milestones: 2.2.x, 2.2.2 Jan 15, 2019

@natemcmaster natemcmaster requested a review from dougbu Jan 15, 2019

@natemcmaster

This comment has been minimized.

Copy link
Member

natemcmaster commented Jan 15, 2019

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

@dougbu

This comment has been minimized.

Copy link
Member

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

@dougbu

dougbu approved these changes Jan 15, 2019

@JamesNK JamesNK merged commit 180f735 into release/2.2 Jan 15, 2019

1 check passed

license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment