Reduce frequency of https check when https is already working

[adamsilverstein]

WordPress currently runs a 'twicedaily' job to check if https support is available. This is wasteful when the user already has https enabled. Although it can happen that a site goes from having https support to not having it (eg. when moving hosts or if an SSL certificate expires, this is probably rare, and the website owner is likely to notice issues when they try accessing their site via https as usual.

Therefore, following the example of the ​Eco-Mode plugin. I propose reducing the frequency in this case to weekly. Tis change will save resources and belongs squarely in the *new* sustainability focus.

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

[adamsilverstein]

As a back of the envelope estimation, this will result in the following energy savings

Assumptions
~80% of sites use https already ​reference
~ 810 Million sites run WordPress ​reference
~ request energy intensity figure is 0.0065 kWh/GB ​reference
~ median page weight is 2MB ​reference

Math
~ 648 million sites will reduce requests from ~60/month to 4/month, a reduction of 56 requests or 36288000000 requests
~ at 2mb average that amounts to 72576000 GB of savings

Total estimated savings of 471,744 KwH/month
~

[oglekler]

I am adding needs-testing to highlight that this patch is ready for review.

[Hareesh Pillai]

While I'm 100% aligned with the objective of this ticket and the potential savings, I'm not entirely sure how to test this patch.
Some details on how to test will help drive this forward.

[audrasjb]

This sounds like a great idea, thanks!

I tried testing this patch on a site on production with https enabled, and it didn't work: the scheduled event never switched to weekly (checking with WP Crontrol).

Also, the PR is still a draft, so let's move this ticket to milestone 6.4.

[adamsilverstein]

Some details on how to test will help drive this forward.

Hi @hareesh-pillai -
Here are some steps to test:

Before and after the patch:

• Set up a site with valid SSL (also, test with invalid SSL if possible)
• Use wp-cli to check when the next wp_https_detection cron job is scheduled. Use this command: wp cron event list, look for the wp_https_detection row
• Note the 'recurrence' column, the expected default value will be 12 hours
• Run the check cron event: wp cron event run wp_https_detection
• Check the cron job list again (wp cron event list)
• Before the patch, the recurrence will remain at 12 hours
• After the patch, if the SSL is valid, the recurrence will change to 1 week

I tried testing this patch on a site on production with https enabled, and it didn't work: the scheduled event never switched to weekly (checking with WP Crontrol).

@audrasjb

Did you leave the patch in place for at least 12 hours? the recurrence will only change to 1 week after the next cron check.

Also, the PR is still a draft, so let's move this ticket to milestone 6.4.

Have to stop making them drafts, I'll update that!

marked as ready for review, only had as a draft out of habit. Appreciate any feedback.

[adamsilverstein]

This could use some unit tests.

[johnbillion]

@adamsilverstein Alternative: Is this cron event really needed? As far as I can tell, the only place a user is informed about support for HTTPS being available when they're using HTTP is when they view Site Health. Could this cron event be removed entirely and the HTTPS check moved to an async site health check?

[oglekler]

This ticket was discussed during a bug scrub.
@adamsilverstein is it possible to follow what @johnbillion is suggested?
@audrasjb can you provide further insides, please?
I want to ping @Clorith for the opinion as well.

Add props to: @mukesh27

[adamsilverstein]

@adamsilverstein Alternative: Is this cron event really needed? As far as I can tell, the only place a user is informed about support for HTTPS being available when they're using HTTP is when they view Site Health. Could this cron event be removed entirely and the HTTPS check moved to an async site health check?

Great suggestion, I will double check that is the case and try moving.

[adamsilverstein]

58494.diff​ removes the cron check entirely, the check is now only executed when visiting Site Health. Also in ​https://github.com/WordPress/wordpress-develop/pull/4582.

I've not tested this yet but I think this will need an upgrade routine to remove the existing cron event. I'll review soon.

I've not tested this yet but I think this will need an upgrade routine to remove the existing cron event. I'll review soon.

Yea, probably: I have this removal code ​here, but that would be much better in an upgrade routine instead. I can look into that or feel free to push a change.

[Michi91]

unschedule by using upgrade routines

[Michi91]

Hey @adamsilverstein I didnt know how to push it to you using git as i dont have access to you repo, but heres a patch :-)

[adamsilverstein]

Hey @adamsilverstein I didnt know how to push it to you using git as i dont have access to you repo, but heres a patch :-)

Great, thanks that works - I'll take a look! (I think you would need to fork my repo, then push to your fork, then open a pr against my branch)

[oglekler]

Hi @Michi91 this is an enhancement, and we have 12 days before Beta 1 and at this moment all enhancements needs to be on the Trunk. Please write if you did what was suggested by Adam to move things forward. Thank you!

[Michi91]

@oglekler thank you for the ping. I have sent the patch as pull request to @adamsilverstein fork and pinged him. Lets see if he can merge :)

[adamsilverstein]

Hey @Michi91 - I wasn't able to use the PR, but the patch worked fine! Thanks! I added this to my PR and will now rebase against trunk to resolve conflicts.

[adamsilverstein]

@johnbillion appreciate a review if you have a moment, this should be ready to go. The code now removes the cron job as part of an upgrade task.

[adamsilverstein]

Thanks for the review, I'll get this committed.

[adamsilverstein]

58494.3.diff​ is a refresh against trunk

[adamsilverstein]

In 56664:

Security: remove the cron event that checked for https support.

Fix an issue where a cron job ran every 12 hours to check for https support - even when https support was already enabled. The check is now run only when the user visits the Site Health page. Reducing the unneeded requests lowers the impact and load of hosting WordPress sites.

The wp_update_https_detection_errors function is deprecated and the https_detection_errors option that was previously set by the cron job is no longer maintained. The pre_wp_update_https_detection_errors filter is deprecated and replaced by the pre_wp_get_https_detection_errors filter which serves the same function.

Props audrasjb, johnbillion, Michi91.
Fixes #58494.

[flixos90]

Related: #62252 was opened due to a remaining use of wp_update_https_detection_errors() which was deprecated here, but that usage wasn't removed.

There's also some related documentation clean-up needed, as pointed out in the PR for that ticket ​https://github.com/WordPress/wordpress-develop/pull/7598/files#r1809152986.

[flixos90]

In 59517:

Site Health: Remove use of deprecated function from wp_is_https_supported().

Follow up to [56664].

Props peter8nss, debarghyabanerjee, sebastienserre, geekofshire, swissspidy, desrosj.
Fixes #62252.
See #58494.