Make WordPress Core

Opened 16 months ago

Closed 6 days ago

#61623 closed defect (bug) (fixed)

Site Health PHP check shows inaccurate status

Reported by: swb1192's profile swb1192 Owned by: westonruter's profile westonruter
Milestone: 6.9 Priority: normal
Severity: normal Version: 6.5.5
Component: Site Health Keywords: has-patch has-screenshots commit
Focuses: administration, performance Cc:

Description

Site Health has a PHP check that returns a string related to the status of the PHP version. It should state if the PHP version is the latest version, an older (but supported) PHP version, or an older and insecure PHP version.

However, the current result states all supported versions are the "current version" of PHP.

To reproduce:

  • Set up a WordPress installation
  • Configure PHP as version 7.4 (supported but insecure and not the latest version)
  • Go to Tools > Site Health
  • View the Passed Tests

See:

"Your site is running the current version of PHP (7.4.30)"

Expected:

"Your site is running on an outdated version of PHP 7.4, which does not receive security updates. It should be updated."

Attachments (3)

Screenshot 2024-07-10 at 12.24.09 PM.png (112.7 KB) - added by swb1192 16 months ago.
screenshot running php 7.4
before patch.jpg (41.4 KB) - added by krupajnanda 13 months ago.
after patch.jpg (137.0 KB) - added by krupajnanda 13 months ago.

Download all attachments as: .zip

Change History (47)

@swb1192
16 months ago

screenshot running php 7.4

#2 @swb1192
16 months ago

  • Keywords close removed

@swissspidy Please re-open this ticket. The issue is not about whether PHP 7.4 is supported or not. The issue is that it says 7.4 is the current version of PHP. It may be recommended, but it's not the current version of PHP.

#3 @swissspidy
16 months ago

I didn't close the ticket. It is still open, the close just says closing is suggested.

#4 @swb1192
16 months ago

Got it, thank you. I can adjust the ticket copy if the intent or issue was unclear.

This ticket was mentioned in PR #7011 on WordPress/wordpress-develop by @psykro.


16 months ago
#5

  • Keywords has-patch added

Updates the Site Health PHP check message to correctly refer to the currently recommended version of PHP, not the current version of PHP

Trac ticket: https://core.trac.wordpress.org/ticket/61623

#6 @psykro
16 months ago

Looking at the code, the text needs to match any version of PHP that is equal to or greater than the recommended version.

So instead of

Your site is running the current version of PHP

It could say

Your site is running at least the minimum recommended version of PHP

This way, if the PHP version exceeds whatever the current recommended version is, the wording still makes sense.

EDIT: the Github bot was quicker than I was, the patch was meant to come after the comment.

Last edited 16 months ago by psykro (previous) (diff)

#7 follow-up: @joemcgill
16 months ago

  • Focuses performance added
  • Milestone changed from Awaiting Review to Future Release

I think clarifying the wording for this health check makes sense. What doesn't make sense is that we are showing this success message also when wp_check_php_version() returns false due to an error validating the /core/serve-happy API response. We could consider changing that behavior as part of this ticket but shouldn't be blocking.

I'm also tagging for the performance focus given that this is labeled as a performance check.

#8 follow-up: @swb1192
16 months ago

@psykro Thanks for the patch. Perhaps better wording could be:

Your site is running a recommended version of PHP.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


16 months ago

#10 @mukesh27
16 months ago

  • Keywords good-first-bug added

#11 @mukesh27
16 months ago

  • Milestone changed from Future Release to 6.7

Hi there!

This ticket was discussed on today's performance bug scrub.

The next step should be update site health copy text.

Move in to 6.7 for visibility.

Additional props: @swissspidy @joemcgill

#12 in reply to: ↑ 8 @psykro
16 months ago

  • Owner set to psykro
  • Status changed from new to accepted

Replying to swb1192:

@psykro Thanks for the patch. Perhaps better wording could be:

Your site is running a recommended version of PHP.

Yup, you're probably right, I'll update the patch.

#13 @SergeyBiryukov
16 months ago

In 58832:

Site Health: Improve the wording for PHP version check.

This aims to make the message more accurate by referring to the version of PHP currently recommended by WordPress, not the current version of PHP.

Follow-up to [44986], [46267], [47254].

Props swb1192, psykro, swissspidy, joemcgill, mukesh27, aristath.
See #61623.

@SergeyBiryukov commented on PR #7011:


16 months ago
#14

Thanks for the PR! Merged in r58832.

#15 @SergeyBiryukov
16 months ago

Thanks everyone! Keeping the ticket open for now to also address comment:7.

#16 @amin7
14 months ago

I have tested the patch.

Test Report

Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/7011.diff


Steps to Test ( As per description)
Set up a WordPress installation
Configure PHP as version 7.4 (supported but insecure and not the latest version)
Go to Tools > Site Health
View the Passed Tests

Environment

WordPress:6.6.1
Browser: Firefox 128.0
PHP: 7.4.3
Theme: TT4
Plugins: none

Before patch

Check the screenshot: https://d.pr/i/5ENYFo

After patch
Check the screenshot https://d.pr/i/KI4zYY

Results
The patch is working as suggested

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


14 months ago

This ticket was mentioned in PR #7350 on WordPress/wordpress-develop by @pbearne.


14 months ago
#18

Refactor the logic for setting the PHP version label and status in the site health check. Adds a condition to handle failures in obtaining the recommended PHP version from WordPress.org, updating the label and description accordingly.

#19 in reply to: ↑ 7 @pbearne
14 months ago

  • Keywords good-first-bug removed
  • Owner changed from psykro to pbearne
  • Status changed from accepted to assigned

Replying to joemcgill:

I think clarifying the wording for this health check makes sense. What doesn't make sense is that we are showing this success message also when wp_check_php_version() returns false due to an error validating the /core/serve-happy API response. We could consider changing that behavior as part of this ticket but shouldn't be blocking.

I'm also tagging for the performance focus given that this is labeled as a performance check.

I have added patch to adjust eh message if the call to wp.org fails

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


13 months ago

@mukesh27 commented on PR #7350:


13 months ago
#21

@joemcgill @SergeyBiryukov Could you please take a look when you have moment so we can commit it before RC.

#22 @krupajnanda
13 months ago

  • Keywords has-screenshots added

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/7350

Environment

  • WordPress: 6.7-alpha-58576-src
  • PHP: 8.2.15
  • Server: nginx/1.25.3
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.15)
  • Browser: Chrome 129.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.2
  • MU Plugins:
    • Safe Autoloaded Options Limit Test (MU Plugin) 1.0
  • Plugins:
    • Test Reports 1.1.0

Expected Results

Text should be updated to : "Your site is running a recommended version of PHP (%s)"

Actual Results

Text is updated after the patch is applied. ✅

Supplemental Artifacts

Add as Attachment

#23 @krupajnanda
13 months ago

@pbearne I am not sure how can I test this scenario:

Your site is running PHP Version (%s) but the call to get the recommended version failed.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


13 months ago

#25 @peterwilsoncc
13 months ago

@SergeyBiryukov @joemcgill Are either of you able to take a quick look at the follow up PR and see if it addresses comment 7

#26 @SergeyBiryukov
13 months ago

  • Milestone changed from 6.7 to 6.8

Would like to do some more testing here, moving to 6.8 for now.

#27 @joemcgill
13 months ago

Agreed. I left some feedback on the PR last week that hasn't all been addressed. I think this needs a bit more thought.

#28 @flixos90
10 months ago

@joemcgill It looks like @pbearne replied to your last feedback. It's not fully addressed, but I think based on his reply it would be great if you could check in again with your proposed approach.

#29 @imranhasanraaz
8 months ago

Test Report

Description

I have tested on 6.8-beta1-59933-src with php 7.4. The text is updated there.
It is showing 'Your site is running a recommended version of PHP (7.4.33)'

Patch tested: REPLACE_WITH_PATCH_URL

Environment

  • WordPress: 6.8-beta1-59933-src
  • PHP: 7.4.33
  • Server: nginx/1.27.2
  • Database: mysqli (Server: 8.0.40 / Client: mysqlnd 7.4.33)
  • Browser: Chrome 133.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.1
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

✅ Text is updated - https://prnt.sc/XqaYQYZSlOCW

Last edited 8 months ago by imranhasanraaz (previous) (diff)

#30 @vgnavada
8 months ago

Hey there!

Is the pull request playground link working properly?
https://playground.wordpress.net/wordpress.html?pr=7350

I tried from 2 different browsers, it's crashing everywhere!

https://img001.prntscr.com/file/img001/VN9sw8z9SeKnTvs7smsn5w.png

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#32 @audrasjb
8 months ago

  • Milestone changed from 6.8 to 6.9

As per today's bug scrub and since 6.8 beta 3 is tomorrow and it's still being discussed, let's move this ticket to 6.9.

This ticket was mentioned in Slack in #core-performance by adamsilverstein. View the logs.


6 months ago

#34 @adamsilverstein
6 months ago

  • Keywords needs-refresh added

#35 @Presskopp
5 months ago

Dashboard:
Your site is running on an outdated version of PHP (8.4.4), which does not receive security updates. It should be updated.

Site Health:
Your site is running on an older version of PHP (8.4.4)

...\wp-admin\includes\dashboard.php:44:
array (size=6)
  'recommended_version' => string '8.4.8' (length=5)
  'minimum_version' => string '7.2.24' (length=6)
  'is_supported' => boolean true
  'is_secure' => boolean false
  'is_acceptable' => boolean false
  'is_lower_than_future_minimum' => boolean false

This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.


13 days ago

@westonruter commented on PR #7350:


13 days ago
#37

@joemcgill Do you feel your feedback has been addressed?

#38 @westonruter
13 days ago

  • Keywords needs-refresh removed
  • Owner changed from pbearne to westonruter
  • Status changed from assigned to reviewing

I think this is good to go. Just pinged a reviewer to confirm concerns have been addressed.

#39 @westonruter
13 days ago

I added a couple comments to the PR with ideas for how it might be improved further.

@westonruter commented on PR #7350:


11 days ago
#40

@pbearne It looks like the last feedback is addressing what Peter found in https://github.com/WordPress/wordpress-develop/pull/7350#pullrequestreview-3396830402

@westonruter commented on PR #7350:


10 days ago
#41

While testing without been able to connect to the WordPress API, I noticed that a passing request to the API does not show the recommended version of WP.

@peterwilsoncc Now it shows as:

https://github.com/user-attachments/assets/e7349ddc-7b4f-48bd-990b-b7d4c6399f5d

@westonruter commented on PR #7350:


10 days ago
#42

When wp_check_php_version() returns:

array(
        'recommended_version' => '10.0.0',
        'minimum_version'     => '9.0.0',
        'is_supported'        => false,
        'is_secure'           => false,
        'is_acceptable'       => false,
)

https://github.com/user-attachments/assets/15a0dcb5-a6c9-462d-9116-f270d19641d9

When:

$response = array(
        'recommended_version' => '10.0.0',
        'minimum_version'     => '8.0.0',
        'is_supported'        => true,
        'is_secure'           => true,
        'is_acceptable'       => true,
);

https://github.com/user-attachments/assets/838c044d-b954-487b-8bb6-5e1bf909c79d

When:

$response = array(
        'recommended_version' => '8.0.0',
        'minimum_version'     => '7.2.0',
        'is_supported'        => true,
        'is_secure'           => true,
        'is_acceptable'       => true,
);

https://github.com/user-attachments/assets/49c9d9f2-1b8b-4499-ba3b-0a43693f49da

#43 @westonruter
10 days ago

  • Keywords commit added

#44 @westonruter
6 days ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 61123:

Site Health: Improve messaging for PHP version status.

Previously, the status could be misleading, especially if the check against the WordPress.org API failed. The initial status message is now more neutral, and a dedicated check has been added to handle cases where the API is unreachable.

Developed in https://github.com/WordPress/wordpress-develop/pull/7350

Follow-up to [58832].

Props pbearne, joemcgill, mukesh27, peterwilsoncc, swb1192, krupajnanda, psykro, SergeyBiryukov, swissspidy, imranhasanraaz, amin7, flixos90, vgnavada, audrasjb, adamsilverstein, Presskopp, westonruter.
Fixes #61623.

Note: See TracTickets for help on using tickets.