Skip to content

Added check for ext-dom to XmlUtil::loadFile #24047

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
Sep 3, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Symfony/Component/Config/Util/XmlUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,14 @@ private function __construct()
* @return \DOMDocument
*
* @throws \InvalidArgumentException When loading of XML file returns error
* @throws \RuntimeException When DOM extension is missing
*/
public static function loadFile($file, $schemaOrCallable = null)
{
if (!extension_loaded('dom')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you pointed out that dom doesn't guarantee xml. Should we put || !extension_loaded('xml') since this is calling xml functions?

Copy link
Contributor Author

@oleg-andreyev oleg-andreyev Aug 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, ext-dom does not guarantee that ext-xml is enabled, but php itself guarantees libxml, so all libxml_* will work, unless php is compiled from source with --disable-libxml (which is probably where uncommon)

$ apt-cache depends php7.0-fpm
php7.0-fpm
  Depends: libmagic1
  Depends: mime-support
  Depends: php7.0-cli
  Depends: php7.0-common
  Depends: php7.0-json
  Depends: php7.0-opcache
  Depends: tzdata
  Depends: ucf
  Depends: init-system-helpers
  Depends: lsb-base
  Depends: libapparmor1
  Depends: libc6
  Depends: libpcre3
  Depends: libssl1.0.0
  Depends: libsystemd0
  Depends: libxml2
  Depends: zlib1g
  Conflicts: <php5-fpm>
  Suggests: php-pear
$ apt-cache depends php7.0-cli
php7.0-cli
  Depends: libedit2
  Depends: libmagic1
  Depends: mime-support
  Depends: php7.0-common
  Depends: php7.0-json
  Depends: php7.0-opcache
  Depends: php7.0-readline
  Depends: tzdata
  Depends: ucf
  Depends: libc6
  Depends: libpcre3
  Depends: libssl1.0.0
  Depends: libxml2
  Depends: zlib1g
  Breaks: <php5-cli>
  Suggests: php-pear
  Replaces: <php5-cli>
$ apt-cache depends php7.0-cgi
php7.0-cgi
  Depends: libmagic1
  Depends: mime-support
  Depends: php7.0-cli
  Depends: php7.0-common
  Depends: php7.0-json
  Depends: php7.0-opcache
  Depends: tzdata
  Depends: ucf
  Depends: libc6
  Depends: libpcre3
  Depends: libssl1.0.0
  Depends: libxml2
  Depends: zlib1g
  Conflicts: <php5-cgi>
  Suggests: php-pear

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok then :)

throw new \RuntimeException('Extension DOM is required.');
}

$content = @file_get_contents($file);
if ('' === trim($content)) {
throw new \InvalidArgumentException(sprintf('File %s does not contain valid XML, it is empty.', $file));
Expand Down