Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109216
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\))
Message-ID: <3F33C0E3-8AE9-4E13-80F1-05E804942488@craigfrancis.co.uk>
Date: Sun, 22 Mar 2020 23:14:30 +0000
Cc: PHP internals <internals@lists.php.net>
To: mike@newclarity.net
Subject: Re: [PHP-DEV] is_literal() + WordPress
From: craig@craigfrancis.co.uk (Craig Francis)

On Sun, 22 Mar 2020 at 19:11, Mike Schinkel <mike@newclarity.net> wrote:
> IMO getting that in WordPress core is highly unlikely



Good point, like all systems, WordPress will need to consider older =
versions of PHP.

But, because this is a new function, they can avoid that issue by using =
`function_exists()`, as in...

    if (function_exists('is_literal') && !is_literal($sql)) {
        trigger_error('This is an unsafe $query, please use =
$wpdb->prepare()', E_USER_NOTICE);
    }

This would be a pretty easy way for WordPress to show they take security =
seriously, and helping developers to avoid these "all too common" =
mistakes.

But I do appreciate how much effort it can be to introduce anything in =
to WordPress :-)

Craig







> On 22 Mar 2020, at 19:10, Mike Schinkel <mike@newclarity.net> wrote:
>=20
>> On Mar 21, 2020, at 7:15 PM, Craig Francis <craig@craigfrancis.co.uk> =
wrote:
>>=20
>> On Sat, 21 Mar 2020 at 22:53, Mike Schinkel <mike@newclarity.net> =
wrote:
>> A large number of PHP users have no control over the platform they =
run on, so the option to use PECL modules is a non-starter for them.
>>=20
>>=20
>> Thanks Mike,
>>=20
>> Personally I agree, I would say PECL modules are not preferable for =
"useful features"; simply because I try to keep my systems only using =
core PHP features where possible (makes server admin easier).
>=20
> +1
>=20
>> As you mention working with WordPress, I've seen a couple of =
developers who have taken examples like:
>>=20
>>    $posts =3D $wpdb->get_results("SELECT ... WHERE =
post_type=3D'post'");
>>=20
>> Then edited it to something dangerous like:
>>=20
>>    $posts =3D $wpdb->get_results("SELECT ... WHERE post_type=3D'" . =
$_GET['type'] . "'");
>=20
> Yes.  That is all too common.
>=20
>> To guard against this, do you think that WordPress could update their =
get_results() function to do something like:
>>=20
>>    public function get_results( $query =3D null, $output =3D OBJECT ) =
{
>>      if (!is_literal($sql)) {
>>        trigger_error('This is an unsafe $query, please use =
$wpdb->prepare()', E_USER_NOTICE);
>>      }
>>=20
>> Perhaps with a better message; then, over the years, increase the =
warning level?
>>=20
>> I think that would be a very useful way of getting developers aware =
of these dangers.
>=20
> Well...
>=20
> IMO getting that in WordPress core is highly unlikely for two (2) =
reasons:
>=20
> ------------
> 1. Getting the WordPress team to adopt new features is about as =
difficult as getting the PHP community to agree on and add a new =
feature. IOW, it is a huge uphill battle.
>=20
> I have many, many unfulfilled feature requests in WordPress' trac that =
are up to 8 years old or more even though on many of the tickets there =
appears to be universal interest.  What I have found is that the =
WordPress team does not address anything unless they need it to =
implement the features that Matt has dictated should be in the next =
version of WordPress. Because of that, many very useful ticketsl sit =
there for years and years and do not get addressed. =20
>=20
> Of course if a core team member with commit access has a pet feature =
they want to add, it typically gets slipped it with no fanfare and then =
we are stuck with it (*cough* Customizer *cough*)
>=20
> I did recently get one ticket addressed about 11 months ago that was a =
subset of a ticket that is 9 years old (and sadly the OP on the older =
ticket has since died of cancer.)
>=20
> But I think I invested probably 80 hours over that duration constantly =
bringing it up on Slack publicly and via DMs and on the ticket. My guess =
is they were just tired of hearing from me.  And ALL my ticket was =
asking for was a damn hook that many plugins had already added because =
WordPress core had not:
>=20
> https://core.trac.wordpress.org/ticket/47056
>=20
> ------------
> 2. More importantly =E2=80=94 and a much higher bar =E2=80=94 =
WordPress core supports PHP 5.6 and so core cannot contain any features =
that are in a later version of PHP.=20
>=20
> =
https://wptavern.com/wordpress-ends-support-for-php-5-2-5-5-bumps-minimum-=
required-php-version-to-5-6
>=20
> And don't expect that to change any time soon. WordPress' minimum PHP =
requirements was notoriously 5.2.4 until March 2019 so it is unlikely =
they will bump the PHP requirement before March 2029. =20
>=20
> (just kidding, but only a bit.)
>=20
> Of course WordPress currently *recommends* PHP 7.3 and most (all?) =
managed hosts support that so userland developers are free to use newer =
version of WordPress on our own projects and in our plugins and themes.=20=

>=20
> https://wordpress.org/about/requirements/
>=20
> But again, WordPress core cannot use any features anytime soon that =
are not in PHP 5.6.