-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix: Prevent overridden PageModel lifecycle methods from being discovered as handlers #64132
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
Conversation
…ered as handlers Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug where overridden PageModel lifecycle methods were incorrectly being discovered as page handlers, causing them to be treated as handler methods with unexpected HTTP method and handler name values.
Key Changes:
- Modified the handler discovery logic to use
GetBaseDefinition()to properly exclude overridden lifecycle methods fromPageModel - Added a regression test to verify that overridden lifecycle methods are not treated as handlers
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Mvc/Mvc.RazorPages/src/ApplicationModels/DefaultPageApplicationModelPartsProvider.cs | Updated the IsHandler method to check the base definition's declaring type, ensuring overridden PageModel lifecycle methods are excluded |
| src/Mvc/Mvc.RazorPages/test/ApplicationModels/DefaultPageApplicationModelProviderTest.cs | Added regression test verifying that overridden lifecycle methods are not discovered as handlers while legitimate handlers like OnGet are still discovered |
src/Mvc/Mvc.RazorPages/test/ApplicationModels/DefaultPageApplicationModelProviderTest.cs
Outdated
Show resolved
Hide resolved
…cationModelProviderTest.cs
Fixes #63692
Problem
When developers override
PageModellifecycle methods (OnPageHandlerExecuting,OnPageHandlerExecuted,OnPageHandlerSelected) in their Razor Pages, these overridden methods were being incorrectly discovered as page handlers. This resulted in them being parsed with unexpected HTTP methods and handler names:OnPageHandlerExecuting→HttpMethod = "Page",HandlerName = "HandlerExecuting"OnPageHandlerExecuted→HttpMethod = "Page",HandlerName = "HandlerExecuted"OnPageHandlerSelected→HttpMethod = "Page",HandlerName = "HandlerSelected"Root Cause
The
IsHandler()method inDefaultPageApplicationModelPartsProviderexcluded methods declared directly onPageModelusing:However, when a developer overrides these methods in a derived class, the
DeclaringTypeis the derived class, notPageModel, causing the check to fail and allowing these lifecycle methods to be treated as handlers.Solution
Changed the exclusion check to use
GetBaseDefinition()to trace back to the original method definition:This ensures that overridden lifecycle methods from
PageModelare properly excluded from handler discovery.Testing
Added regression test
PopulateHandlerMethods_Ignores_OverriddenPageModelLifecycleMethodsthat verifies:OnPageHandlerExecuting,OnPageHandlerExecuted,OnPageHandlerSelected) are NOT discovered as handlersOnGetcontinue to be discovered correctlyAll existing tests pass, confirming no behavioral changes for legitimate handler methods.
Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.