Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Some server administrators may choose to disable the PHP function phpinfo() for security reasons (see https://www.drupal.org/node/243993). SystemInfoController::php() does not verify that phpinfo exists before using it. Potentially this could create php fatal error in systems where phpinfo is disabled
Proposed resolution
Add a check in SystemInfoController::php()
to ensure that phpinfo() exists.
Remaining tasks
create patch.
User interface changes
none.
API changes
none.
Comment | File | Size | Author |
---|---|---|---|
#4 | d8-system-info-controller-phpinfo-check-2465467-4.patch | 910 bytes | willzyx |
#2 | d8-system-info-controller-phpinfo-check-2465467-2.patch | 1017 bytes | Dom. |
#1 | d8-system-info-controller-phpinfo-check-2465467-1.patch | 1013 bytes | willzyx |
Comments
Comment #1
willzyx CreditAttribution: willzyx commentedComment #2
Dom. CreditAttribution: Dom. commentedHi !
- patch review for Drupal standard
- patch manually applied and checked
Therefore, patch #1 is RTBC+1 !
However I would suggest the following, thus not applying the RTBC tag. If you don't agree with the following, please apply the RTBC tag since #1 is ok :
- add "visit" in the last sentence since otherwise it is a fragment (looks weird that there are no verb, but I'm not native english so I'm probably wrong.)
For more information, visit <a href="@phpinfo">Enabling and disabling phpinfo()</a> handbook page.
- remove the empty lign before return statement.
Comment #3
andypostNot sure that the text makes sense, you suggest to revert security setting on server to enable
phpinfo()
that disabled for reason.The link to d.o page makes a lot of sense, so I think text should be like.
Suppose
The phpinfo() function has been disabled for security reasons. For more info....
Comment #4
willzyx CreditAttribution: willzyx commented@Dom.
- it makes sense
- on coding standards there is no reference of the standard to follow for this thing. it's just a matter of preference. For me the change you made is fine
@andypost
It's the same sentence used in the status report page (see
system_requirements()
); we suggest to revert setting on server to enable phpinfo() IF you want to see your server's phpinfo() information.However, the requested change makes sense
Comment #5
Dom. CreditAttribution: Dom. commentedRTBC+1
Comment #6
webchickHuh. Had no idea this was an option. You learn something new every day. :)
Can't envision a useful way to automated test this, so...
Committed and pushed to 8.0.x. Thanks!