Adding a disable_functions test for set_time_limit function.

[zeleny]

There is a lots of shared hosting servers, which one has disabled set_time_limit function by php.ini. So usually that makes problems with updating and maintaining the WordPress.

I've made a if test for all set_time_limit I've found at the source, so they do not trigger if that function is disabled by php.ini.

<?php
if (strpos(ini_get('disable_functions'), 'set_time_limit') === 0){
    set_time_limit($some_time);
}

​https://github.com/jokerrs/wordpress-develop/commit/bfb135be80d8e86467f6052856cfdac25014b3b1

There is a lots of shared hosting servers, which one has disabled set_time_limit function by php.ini. So usually that makes problems with updating and maintaining the WordPress.
I've made a if test for all set_time_limit I've found at the source, so they do not trigger if that function is disabled by php.ini.

<?php
if (strpos(ini_get('disable_functions'), 'set_time_limit') === 0){
    set_time_limit($some_time);
}

Trac ticket: #57248

[zeleny]

Changed code to

<?php
if ( strpos( ini_get( 'disable_functions' ), 'set_time_limit' ) !== 0 ) {
    set_time_limit($some_time);
}

Code changed

{{{php
<?php
if ( strpos( ini_get( 'disable_functions' ), 'set_time_limit' ) !== 0 ) {

set_time_limit($some_time);

}
}}}

Hi @jokerrs,

thanks for your PR! I haven't looked into whether this is necessary, but would like to share some feedback on your PHP code:
Comparing the strpos return value for 0 would mean that you are only looking for set_time_limit at the beginning of that ini directive. As other PHP functions could also be disabled, and could come before set_time_limit in that list, you would have to compare to false.

Then however, you could use the more modern PHP function ​`str_contains()`. While that was only added in PHP 8, WordPress has been polyfilling that function since WP 5.9.

It might also worthfile to consider using function_exists() here instead, as that also seems to catch functions that are in the disable_functions directive. A quick search in the WordPress codebase seems to indicate that this might be the preferred way, as there's actually just one match for disable_functions.

Best wishes,
Tobias

Hi @TobiasBg,

I was mainly goin to use the if ( function_exists( 'set_time_limit' ) ) { but, I've saw already in a WordPress code (wp-includes/PHPMailer/SMTP.php:435) the one version I've used in this pull request. That's why is that format that kind of format.

Hi,

Yes, that example that you found is however in PHPMailer, an external library.
This also means that files of external libraries (PHPMailer, ID3, and maybe also that class-pop3.php file) should not be modified. Instead, desired changes would have to be reported to the external library's respective repo or developer.

Besides that match in PHPMailer, there seems to be just one other occurrence of disable_functions (and another one in a code comment where function_exists() is however used), but many more of function_exists(), the WordPress core committers might like that approach more. There's also https://core.trac.wordpress.org/ticket/37680#comment:18 which seems to discuss this a little bit.

Best wishes,
Tobias

Thank you @TobiasBg for your reply,

To be honest, I didn't search the repository for the function_exists, only for the set_time_limit.
But, you are right, the function_exists is a better way of approaching the problem.

About the external libraries, It's my fault I didn't suggest their change but here.

Anyway, the set_time_limit problem is a real thing, because of the many exploits of that function on the shared hosting servers.
That's why lots of hosting providers have that function disabled by default in php.ini.
And using that function should be "guarded" by function_exists in my opinion.

Hi @jokerrs,

no worries, you are doing nothing wrong! :-) I'm just trying to help bringing your suggested patch into a good shape so that the core committers can review it without much hassle if they agree that this is something that is worthwhile adding to WordPress!

Thanks again for your contribution!

Best wishes,
Tobias

[SergeyBiryukov]

Hi there, welcome to WordPress Trac! Thanks for the report.

This was previously reported in #55711, and that ticket already has some discussion on using function_exists() vs. checking ini_get( 'disable_functions' ).

Personally, I agree that function_exists() would be correct here, but let's continue in the other ticket to keep the discussion in one place. I'll edit the PR description to attach it to that ticket.

Please read the thread in https://core.trac.wordpress.org/ticket/55711 - function_exists() is not a good way to do this.

Thanks for the PR! Merged in r55258.

As noted in the comments on the ticket, we can follow up later if an alternative to function_exists() should be considered.