Avoid HTTP request to load XML Schema during validation of XML configuration file

[staabm]

Q	A
PHPUnit version	12.1.0
PHP version	8.4.5
Installation Method	phive

per report in infection/infection#2303 it seems phpunit is using a non local path for the schema.xsd which means running phpunit only works with a online connection (when infection triggers schema validation).

We need to use infection from a gitlab CI which have access to the internet behind a proxy. In the CI we set http_proxy and https_proxy env vars (with the proxy url). It's working for tools like composer (composer install downloads dependencies correctly for example). Infection try to validate the phpunit.xml.dist xml file with DOMDocument::schemaValidate and, as the xsi:noNamespaceSchemaLocation attribute value is the external url https://schema.phpunit.de/12.1/phpunit.xsd infection need to access to it through the proxy server.

We can see that in this commit : 0385463 if phpunit is installed with composer, the default value of xsi:noNamespaceSchemaLocation is the local phpunit.xsd file. But in the project, we install phpunit with phive. So the default xsd is https://schema.phpunit.de/12.1/phpunit.xsd.

I have no experience with phive and cannot confirm the claim made though.

in case the initial reported problem really exist, would it be possible to change the path to the schema xsd to a local path?

[sebastianbergmann]

I am sorry, but I do not understand what you want to report.

"installed via phive" should not matter, the difference is between "PHPUnit installed using Composer" and "using PHAR of PHPUnit".

The changes made in 0385463 only affect the XML configuration file generator (--generate-configuration), not the XML configuration file loader.

[sebastianbergmann]

I cannot see a bug here.

If your XML configuration file for PHPUnit starts with

<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/12.3/phpunit.xsd"

then validating your XML configuration requires access to https://schema.phpunit.de/12.3/phpunit.xsd.

This "problem" can be avoided when using a Composer install of PHPUnit by using xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd".

When you use PHPUnit from its PHAR then you do not have a local copy to reference in your XML configuration file for PHPUnit.

The PHAR for PHPUnit contains all XSD files for PHPUnit XML configuration files since PHPUnit 8.5. So we could replace https://schema.phpunit.de/12.3/phpunit.xsd, for example, with the path to the bundled XSD file for PHPUnit 12.3. I am not yet convinced, though, whether such a change should be implemented. If it were implemented, it would be an enhancement, not a bug fix.

[staabm]

I cannot see a bug here.

yeah.. bug was the wrong wording.

The PHAR for PHPUnit contains all XSD files for PHPUnit XML configuration files since PHPUnit 8.5. So we could replace https://schema.phpunit.de/12.3/phpunit.xsd, for example, with the path to the bundled XSD file for PHPUnit 12.3. I am not yet convinced, though, whether such a change should be implemented. If it were implemented, it would be an enhancement, not a bug fix.

yeah I think thats what I had in mind to allow the xml validation to work offline

[sebastianbergmann]

We already do what I described above:

phpunit/src/TextUI/Configuration/Xml/Loader.php

Lines 107 to 121 in 264da86

	try {
	$xsdFilename = (new SchemaFinder)->find(Version::series());
	} catch (CannotFindSchemaException $e) {
	throw new Exception(
	$e->getMessage(),
	$e->getCode(),
	$e,
	);
	}
	$configurationFileRealpath = realpath($filename);
	assert($configurationFileRealpath !== false && $configurationFileRealpath !== '');
	$validationResult = (new Validator)->validate($document, $xsdFilename);

If the code above works as intended, then the only thing that would trigger an HTTP request for fetching the schema, at least as far as I can see, would be DOMDocument::loadXML(). However, as far as I know, just loading XML using that method does not trigger validation.

[theseer]

However, as far as I know, just loading XML using that method does not trigger validation.

We're using Schema validation and that is not performed at load time let alone automagically by the DOM extension. It might be different for the new XMLDomDocument implemented by @nielsdos, but we're not using that yet.

Technically though, whether or not a network access happens depends a bit on the options flag specified and what type of operation we want done at load time. As far as I know, there is no way to automagically have a Schema based validation happen but we could specify replacing of entities or DTD based validation. I'm not aware of us doing that anywhere though.

To disallow any network access for any reason, we could use ::load with the LIBXML_NONET option set. Again, that should not be needed as we do not use any feature (to the best of my knowledge) that would need network access.

To me though, reading the original issue description from @staabm, this looks more like a bug in infection: Instead of using PHPUnit's internal mechanism - which would use a local file either way - it sounds like infection tries to validate the configuration by (blindly?) making a network call based on the URL in an XML document. That is dangerous at best and of course may or may not work as expected.

[nielsdos]

Just loading the document does not trigger validation, not in the old classes and not in the new classes. After all, doing that behind the back of the developer may be dangerous. You have to make an explicit API call to validate an XML document against a schema.

[theseer]

Thanks for clarifying Niels :-)

[sebastianbergmann]

Instead of using PHPUnit's internal mechanism - which would use a local file either way - it sounds like infection tries to validate the configuration by (blindly?) making a network call based on the URL in an XML document.

PHPUnit's internal mechanism is exactly that: internal. And I would rather not change that. But I agree: Infection should use a similar approach in their code.

[staabm]

Thanks dor the feedback everyone. I will take a closer look