magento2 icon indicating copy to clipboard operation
magento2 copied to clipboard

Allow only valid preferences during setup:di:compile

Open fredden opened this issue 3 years ago • 48 comments

Description

The setup:di:compile command has exclude lists to avoid loading / compiling dependency injection for 'test' classes. When preferences exist for these excluded classes, any plugins associated with the original classes are (potentially) rendered useless. This is because the child class (preference) can call the original class in a way which does not use Magento's plugin system (ie, parent::methodName()). And it's impossible to plugin a class that does not go through Magento's compilation process (to have interceptors created, etc). See https://github.com/magento/security-package/issues/296 for a real-world example of this problem in action.

This pull request began as a change to the regular expressions used to exclude classes, and has grown into expanding the exclude list to cover preferences. Changes include:

  • Removal of invalid preferences already within the code-base.
  • Alerting (exception) for preferences that do not exist.
  • Alerting (exception) for preferences that do not exist because of the existing exclude lists.
  • Various bug-fixes for setup:di:compile-related classes.

Related Pull Requests

  • https://github.com/magento/inventory/pull/3315
  • https://github.com/magento-commerce/magento2-page-builder/pull/304

Fixed Issues (if relevant)

  1. Fixes https://github.com/magento/security-package/issues/296

Manual testing scenarios

  1. Create an "around" plugin on Magento\TwoFactorAuth\Observer\ControllerActionPredispatch::execute()
  2. Notice this works in developer mode, but not in production mode

Questions or comments

None

Contribution checklist

  • [x] Pull request has a meaningful description of its purpose
  • [x] All commits are accompanied by meaningful commit messages
  • [x] All new or changed code is covered with unit/integration tests (if applicable)
  • [x] README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • [ ] All automated tests passed successfully (all builds are green)

fredden avatar Jun 07 '21 09:06 fredden