Skip to content

Conversation

@adrian-martinez-interactiv4
Copy link
Contributor

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 commented Oct 30, 2017

When saving custom layout update xml in CMS page, in production mode, xml is not validated against schema file, due to \Magento\Framework\App\Arguments\ValidationState::isValidationRequiredmethod:

    /**
     * Retrieve current validation state
     *
     * @return boolean
     */
    public function isValidationRequired()
    {
        return $this->_appMode == \Magento\Framework\App\State::MODE_DEVELOPER;
    }

Actual result:
captura de pantalla 2017-10-30 a las 3 46 56
captura de pantalla 2017-10-30 a las 3 47 15

Desired behaviour would be check the layout update xml against the schema, and show error message to the user if it is invalid:
captura de pantalla 2017-10-30 a las 3 58 10

Also, when layout update is validated, validation exceptions are unhandled, showing a report error in production mode, and showing this error in developer mode:
captura de pantalla 2017-10-30 a las 1 03 01

For example, uncomment demo layout handle in home page:
captura de pantalla 2017-10-30 a las 1 02 34

Desired behaviour would be handle the exception properly instead of crash, and show error message to the user:
captura de pantalla 2017-10-30 a las 1 26 21

Description

  • According to doc block, \Magento\Cms\Controller\Adminhtml\Page\PostDataProcessor::validate should always return a boolean value. It uses \Magento\Framework\View\Model\Layout\Update\Validator::isValid method, that may throw an exception if xml is not valid, so \Magento\Cms\Controller\Adminhtml\Page\PostDataProcessor::validate must handle possible exceptions thrown by xml validator, in order to always return a boolean value.
  • \Magento\Cms\Model\Page\DomValidationState has been added, and it is injected from post data processor to force layout update xml validation, only upon cms page save.

This PR include changes of PR #11866

Manual testing scenarios

  1. Enable developer mode

  2. Edit or create a CMS page, enter some invalid xml in Layout Update XML textarea

  3. Try to save the page, application crashes

  4. Enable production mode

  5. Edit or create a CMS page, enter some invalid xml in Layout Update XML textarea

  6. Try to save the page, xml does not get validated

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@dmanners dmanners self-assigned this Oct 30, 2017
@adrian-martinez-interactiv4 adrian-martinez-interactiv4 force-pushed the FR21#CMS-PAGE-HANDLE-LAYOUT-UPDATE-XML-VALIDATION-EXCEPTION branch from ca9eabb to 99c70e0 Compare October 30, 2017 13:57
@adrian-martinez-interactiv4 adrian-martinez-interactiv4 changed the title [Backport 2.1-develop] CMS Page - Handle layout update xml validation exceptions [Backport 2.1-develop] CMS Page - Force validate layout update xml in production mode when saving CMS Page - Handle layout update xml validation exceptions Oct 30, 2017
@dmanners
Copy link
Contributor

dmanners commented Nov 1, 2017

Will put this on hold until the PR to 2.2-develop is merged. Then we can process the backports.

CMS Page - Force validate layout update xml, even in production mode, when saving CMS Page
@adrian-martinez-interactiv4 adrian-martinez-interactiv4 force-pushed the FR21#CMS-PAGE-HANDLE-LAYOUT-UPDATE-XML-VALIDATION-EXCEPTION branch from d673d7d to 74ba7a0 Compare November 30, 2017 16:33
@dmanners dmanners added this to the December 2017 milestone Dec 1, 2017
@magento-team magento-team merged commit db9ad08 into magento:2.1-develop Dec 4, 2017
magento-team pushed a commit that referenced this pull request Dec 4, 2017
…t update xml in production mode when saving CMS Page - Handle layout update xml validation exceptions #11860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants