Skip to content

Throw ValidationException for invalid xml #12859

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 1 commit into from
Dec 28, 2017

Conversation

pmclain
Copy link
Contributor

@pmclain pmclain commented Dec 22, 2017

This change ensures the offending file path is output in the error
report.

Description

Currently, when malformed $content is fed into \Magento\Config\Model\Config\Structure\Reader::processingDocument and unhandled \Exception is thrown. The output fails to indicate the location of the offending file, example:

1 exception(s):
Exception #0 (Exception): Warning: DOMDocument::loadXML(): Opening and ending tag mismatch: system line 3 and section in Entity, line: 29 in /path2magento/vendor/magento/module-config/Model/Config/Structure/Reader.php on line 133

The proposed change catches the \Exception and throws \Magento\Framework\Config\Dom\ValidationException instead. This still causes the application to report an error, but logs the offending file in the error report, example:

1 exception(s):
Exception #0 (Magento\Framework\Exception\LocalizedException): Invalid XML in file /path2magento/app/code/Vendor/Module/etc/adminhtml/system.xml:
Warning: DOMDocument::loadXML(): Opening and ending tag mismatch: system line 3 and section in Entity, line: 29 in /path2magento/vendor/magento/module-config/Model/Config/Structure/Reader.php on line 133

Manual testing scenarios

  1. Clear config cache
  2. Malform any system.xml file, such as a duplicate closing tag.
  3. Log in to the admin panel and attempt to visit the configuration page.
  4. The application will return a 503 response and will include the path to the bad system.xml file in the error report.

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)

This change ensures the offending file path is output in the error
report.
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.

3 participants