Make phpinfo on the admin status report configurable [D7]

The last submitted patch, 4: 3358515-4.patch, failed testing. View results →

Testbot seems to be a bit flaky today for some reason; have had to rerun a couple of tests.

Thanks! Not sure which of these two issues would go first, but we should probably extend/update the basic test of phpinfo page ( 📌 Add test(s) for SA-CORE-2023-004 [D7] Fixed ) and check also the behavior of that page in case the variable sa_core_2023_004_phpinfo_flags is changed - for example when the variable is reverted to the state before SA-CORE-2013-004 (e.g. empty string), if the removed section will be back.

It can be done here or in the mentioned issue (it depends on the order how we would like to proceed).

Yes I agree; let's commit the basic test first then we can add testing of the new variable here.

I have tried to configure the PHP info by this way under modules/system/system.admin.inc:

function system_php() {
$phpinfo_flags = variable_get('sa_core_2023_004_phpinfo_flags');
$phpinfo_flags = empty($phpinfo_flags) ? INFO_ALL : $phpinfo_flags;
phpinfo($phpinfo_flags);
drupal_exit();
}

In our default.settings.php we can add some comments like:

/**
* Configuration for phpinfo() admin status report.
*
* Drupal's admin UI includes a report at admin/reports/status/php which shows
* the output of phpinfo(). The full output can contain sensitive information
* so by default Drupal removes some sections.
*
* This behaviour can be configured by setting this variable to a different
* value corresponding to the flags parameter of phpinfo().
*
* If you need to expose more information in the report - for example to debug a
* problem - consider doing so temporarily.
*
* INFO_GENERAL The configuration line, php.ini location, build date, Web Server, System and more.
* INFO_CREDITS PHP Credits. See also phpcredits().
* INFO_CONFIGURATION Current Local and Master values for PHP directives. See also ini_get().
* INFO_MODULES Loaded modules and their respective settings. See also get_loaded_extensions().
* INFO_ENVIRONMENT Environment Variable information that's also available in $_ENV.
* INFO_VARIABLES Shows all predefined variables from EGPCS (Environment, GET, POST, Cookie, Server).
* INFO_LICENSE PHP License information. See also the » license FAQ.
* INFO_ALL Shows all of the above.
*
* By default INFO_ALL is enabled.
* You can use above parameters to configure the phpinfo for example:
* (INFO_ENVIRONMENT | INFO_MODULES)
* For More information check the link below.
* @see https://www.php.net/manual/function.phpinfo.php
*/
#$conf['sa_core_2023_004_phpinfo_flags'] = (INFO_ALL);

Thanks for the suggestion, but I think it's better to include the link to the PHP documentation rather than reproduce a lot of that information in the comments here.

default.settings.php is already a massive wall of text, plus the documentation may change over time.

Also better to stick to the very well established pattern of providing a default in the call to variable_get().

Now that the test has been committed we can add to it to verify that the new variable works.

I think this looks good, thanks! Tests are green and I have also tested this manually via settings.php change and it worked correctly. Moving to RTBC. Not sure if we need to wait for the D10 issue though.

#11 looks good now and works well. Thanks

We should add a change record before commit. D10 draft CR is here: https://www.drupal.org/node/3360166 →

Re:

Not sure if we need to wait for the D10 issue though.

We have decided to wait just before the D7 release and then will see if we commit this or wait for the D10 issue to get first.

The D10/11 issue has been committed, so we're good to get this in before the next D7 release.

I'll create a D7 CR.

Committed, thanks!

Added also release notes snippet (inspired by D10).

@poker10, I noticed a few fails in the tests, maybe just re-trigger them to fix that.

PDOException: SQLSTATE[42S02]: Base table or view not found: 1051 Unknown table 'jenkins_drupal_d7_262033.simpletest656510cache_page' in /var/www/html/includes/database/database.inc:2284

https://www.drupal.org/pift-ci-job/2676982 →

Thanks, will check that. There was last push few minutes ago, so tests are running again. But these exceptions seems like the classic random failures, which we unfortunatelly have in D7 tests from time to time (Table XXX already exists, Base table or view not found).

Seems good, all tests are green.

@poker10 , yes all green again, thanks :)

Automatically closed - issue fixed for 2 weeks with no activity.