[RFC] Deprecations for PHP 7.3

On Sun, Jun 24, 2018 at 10:29 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:

This RFC collects a number of deprecations for PHP 7.3 which I consider
to
be too minor to warrant a separate proposal. However, each deprecation
will
still be voted separately.

https://wiki.php.net/rfc/deprecations_php_7_3

Thanks. FWIW, I'm +1 on all 4 proposals.

If you have a minor deprecation in mind, but were too lazy to write an
RFC
for it, please write me a mail until tomorrow, so that it might be
included
as part of this proposal. As time is limited I don't want to include
anything larger or controversial in this RFC though.

I've recently submitted https://github.com/php/php-src/pull/3322 to
deprecate FILTER_FLAG_SCHEME_REQUIRED and FILTER_FLAG_HOST_REQUIRED. I
don't think this needs an RFC, but it might be something which could be
added to this RFC?

I've added these deprecations to the RFC. Please feel free to update the
description if you'd like to add something.

I think it would have been fine to also land these deprecations directly
(as you already wrote to internals about it in the past), but as the have
the opportunity, we might as well go through the motions.

Nikita

On Sun, Jun 24, 2018 at 10:29 PM, Christoph M. Becker cmbecker69@gmx.de
wrote:

This RFC collects a number of deprecations for PHP 7.3 which I consider
to
be too minor to warrant a separate proposal. However, each deprecation
will
still be voted separately.

https://wiki.php.net/rfc/deprecations_php_7_3

Thanks. FWIW, I'm +1 on all 4 proposals.

If you have a minor deprecation in mind, but were too lazy to write an
RFC
for it, please write me a mail until tomorrow, so that it might be
included
as part of this proposal. As time is limited I don't want to include
anything larger or controversial in this RFC though.

I've recently submitted https://github.com/php/php-src/pull/3322 to
deprecate FILTER_FLAG_SCHEME_REQUIRED and FILTER_FLAG_HOST_REQUIRED. I
don't think this needs an RFC, but it might be something which could be
added to this RFC?

Procedural question: Would you prefer that the vote for this (and the other
two deprecation RFCs) is one week (the minimum voting period), or that the
vote is two weeks but the deprecations land for beta 2 rather than beta 1?

Nikita

fgetss() function and string.strip_tags filter

I think I disagree with "strip_tags() itself, due to its limitations and
known bugs, already has very few legitimate applications" and certainly
the manual does not have any notice to that effect. I am not sure what
is the reason to remove this functionality, would like to hear more
about it - and it doesn't seem so minor as to be 1/4 of the RFC. If I
had to vote today, I'd probably vote no just on this point.
It may be true that strip_tags() alone without streaming part could be
implemented easier. However, that is not a reason to drop functionality
by itself, unless it's already completely broken. If it is, I'd like to
hear more about it.

There are multiple bug reports regarding strip_tags()'s broken behavior
on (slightly) malformed HTML, e.g. https://bugs.php.net/63212,
https://bugs.php.net/64430 and https://bugs.php.net/74371, which
renders the function unusable on arbitrary user supplied input.

--
Christoph M. Becker

Defining a free-standing assert() function

Eww, yeah. I can see that problem. I'm not a huge fan of banning all
namespaced assert() function declarations, but it's certainly the most
direct "solution" to the problem. A very quick scan of github only
shows one effected usage in a repo with no followers/stars apart from
the owner. Given the lack of whole-program analysis, it's either ban
the declarations, or abandon elision. Frankly, the number of people
taking advantage of the free dev-only check is probably way higher
than the number trying to define a function called 'assert' so.... ?

Or require a FQN for an imported assert()? Importing functions is getting
traction.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On Sun, Jun 24, 2018 at 11:47 AM, Nikita Popov nikita.ppv@gmail.com
wrote:

https://wiki.php.net/rfc/deprecations_php_7_3

Undocumented mbstring function aliases

Yeah, if they're just dumb aliases, then it's a slight gain to narrow
the symbol table by removing duplicates. A modest composer package
(nay, include file) would be sufficient to provide a bridge for
existing projects which use the removed aliases once they're gone.
This deprecation isn't needed, but it's also fairly low impact. ?

String search functions with integer needle

Agree with stas, the wtf quotient on this one is high. Mitigating
this is the availability of strict types, but even still I think
deprecating the behavior altogether is a wise move. Callers can
easily insert a call to chr() (or IntlChar::chr() ) to maintain the
old behavior in a version agnostic way. ?

fgetss() function and string.strip_tags filter

"...these functions seem to be of very little utility..." [citation needed]
Anecdotally I struggle to imagine the use-case, but my imagination
isn't what it once was. There's not a trivial workaround for someone
who's dealing with a large/streaming data source from which they want
to strip tags. Deprecating this should come at a higher standard of:
proving that it is used, providing a userspace replacement, or both.
?

Some background here. Originally this RFC suggested to deprecate
strip_tags() entirely, but based on early feedback I decided not to.

The original motivation for strip_tags() was basically that the function
has few legitimate applications, but is easy to use in the wrong way. The
two misuses I have seen often enough to motivate a deprecation are:

a) Without allowed_tags: People using strip_tags() where they really just
want htmlspecialchars(). It's obvious to you and I that this is a bad idea,
but there are really people out there who do this. I've actually had
discussions on StackOverflow with people who did not want to believe that,
yes, using htmlspecialchars() is safe and they do not need to use
strip_tags() to eliminate XSS threats. Using strip_tags() for this purpose
is made worse by the fact that it cuts off all text after a standalone <.

b) With allowed_tags: People thinking that using this is safe. Because
attributes on allowed tags are preserved, strip_tags() in this mode
provides zero safety guarantees. The correct alternative is to use
something like HtmlPurifier.

However, there are also not entirely illegitimate applications:

a) Without allowed_tags: Convert HTML to plain-text for use in emails. This
is not a good solution, but it's a quick&dirty way to make things work.

b) With allowed_tags: In combination with WYSIWYG editors that generate
certain HTML tags and under the assumption that the input is completely
trusted. The use of strip_tags() here is more about preventing the users
from shooting themselves in the foot than about security.

Of course, there are also the implementation issues with strip_tags. I
already mentioned that one problem is that all text after < is cut off and
cmb pointed to some more bugs. This makes strip_tags() over-aggressive in
the cases where it's use might generally be legitimate.

This RFC goes the conservative route of only deprecating the streaming
functionality, which is the main source of complexity in the
implementation, while also being the least useful aspect of it.

Defining a free-standing assert() function

Eww, yeah. I can see that problem. I'm not a huge fan of banning all
namespaced assert() function declarations, but it's certainly the most
direct "solution" to the problem. A very quick scan of github only
shows one effected usage in a repo with no followers/stars apart from
the owner. Given the lack of whole-program analysis, it's either ban
the declarations, or abandon elision. Frankly, the number of people
taking advantage of the free dev-only check is probably way higher
than the number trying to define a function called 'assert' so.... ?

TBH I'd be fine with just ignoring this issue. I only included this because
I got some loud complaints from some users who tried to do something like
this and ran into the issue.

Nikita

On Sun, Jun 24, 2018 at 11:47 AM, Nikita Popov nikita.ppv@gmail.com
wrote:

https://wiki.php.net/rfc/deprecations_php_7_3

Undocumented mbstring function aliases

Yeah, if they're just dumb aliases, then it's a slight gain to narrow
the symbol table by removing duplicates. A modest composer package
(nay, include file) would be sufficient to provide a bridge for
existing projects which use the removed aliases once they're gone.
This deprecation isn't needed, but it's also fairly low impact. ?

String search functions with integer needle

Agree with stas, the wtf quotient on this one is high. Mitigating
this is the availability of strict types, but even still I think
deprecating the behavior altogether is a wise move. Callers can
easily insert a call to chr() (or IntlChar::chr() ) to maintain the
old behavior in a version agnostic way. ?

fgetss() function and string.strip_tags filter

"...these functions seem to be of very little utility..." [citation needed]
Anecdotally I struggle to imagine the use-case, but my imagination
isn't what it once was. There's not a trivial workaround for someone
who's dealing with a large/streaming data source from which they want
to strip tags. Deprecating this should come at a higher standard of:
proving that it is used, providing a userspace replacement, or both.
?

As an insight into how much this functionality is used from a somewhat
unexpected direction: While implementing the deprecation, I found out that
the string.strip_tags filter does not have a single test and has been
segfaulting on construction since PHP 7.0 if no allowed tags are specified
(and performed OOB reads if they were specified). These issues have been
fixed in
https://github.com/php/php-src/commit/791f07e4f06a943bd7892bdc539a7313fb3d6d1e
.

Nikita

Hi

Den søn. 24. jun. 2018 kl. 18.47 skrev Nikita Popov <nikita.ppv@gmail.com

:
If you have a minor deprecation in mind, but were too lazy to write an
RFC
for it, please write me a mail until tomorrow, so that it might be
included
as part of this proposal. As time is limited I don't want to include
anything larger or controversial in this RFC though.

As suggested in the past, I would like to add the following to this
list (if its not too late):

• The (real) type-cast and its function, is_real(). There doesn't
  seem to be any support for reals in settype() anyway (side note: in
  the implementation of settype() it claims "double" is deprecated)
• Function variants that already exists as constants, php_sapi_name()

PHP_SAPI, pi() > M_PI, phpversion() > PHP_VERSION etc

• enable_dl ini directive, as dl() is only operational for CLI and Embed
  anyway
• pdo_odbc.db2_instance_name ini directive, its been marked as
  deprecated and mentioned in the manual since PHP 5.1.11 that it will
  certainly be removed in the future

Feel free to add this one to the RFC, if you know what the background for
the deprecation is. Given that the manual already marks it as deprecated
for so long, this seems like a mere technicality.

• The 'b' constant string prefix, its not used and was meant as to
  make code ready for PHP6, should the time come where we want to add a
  feature that uses this, we can always re-add it

Other things thats been suggested by others in the past:

• Second parameter of spl_autoload() and its associated function
  spl_autoload_extensions()
• hebrevc() -- same as hebrev() + nl2br(), perhaps even deprecate
  hebrev() too (see next one)
• convert_cyr_string() -- Point to mb_convert_encoding() / iconv
• Remove get_magic_quotes_gpc(), not been working for what, 5+ years now?
• allow_url_include ini option, this can lead to all kinds of
  security complications anyway
• The alternative string interpolation syntaxes (${varName},
  ${varName['offset']}, ${expr}) and make them more consistent
  ({$varName}, {$varName['offset']}, {${expr}})
• The historial parameter handling that works both ways for
  implode(), should be unified to match that of explode()

I could probably think of more, but thats all I got for now. I can and
will of course help produce the relevant patches should it be needed
for most of these things.

Given the recent discussions about having a PHP 7.4 release for additional
deprecations, I'm going to punt on the rest. While they may be no-brainers
to you and I, a lot of those are actually quite controversial (e.g. there
was an RFC to deprecate b' in 7.2 and it was declined -- go figure!)

I'd suggest that you already open up the next deprecation RFC, to make sure
these are not forgotten. Some of them should probably get separate
proposals altogether (especially the var interpolation syntax).

Nikita

Hi!

• The (real) type-cast and its function, is_real(). There doesn't
  seem to be any support for reals in settype() anyway (side note: in
  the implementation of settype() it claims "double" is deprecated)
• Function variants that already exists as constants, php_sapi_name()

PHP_SAPI, pi() > M_PI, phpversion() > PHP_VERSION etc

Weighting bc preak potential vs. improvement benefit, I am not sure it's
worth it for both. Maybe for "real" it's ok as I haven't really seen
anybody using it in ages, looks like most Fortran programmers either
retired or aren't using PHP :)

• enable_dl ini directive, as dl() is only operational for CLI and Embed anyway

Makes sense.

• The 'b' constant string prefix, its not used and was meant as to
  make code ready for PHP6, should the time come where we want to add a
  feature that uses this, we can always re-add it

Yeah this one we probably just have to remove, it doesn't do anything now.

Other things thats been suggested by others in the past:

• Second parameter of spl_autoload() and its associated function
  spl_autoload_extensions()

Why?

• hebrevc() -- same as hebrev() + nl2br(), perhaps even deprecate
  hebrev() too (see next one)

Probably less useful now that browsers finally can render bidi texts
properly, but may be still useable for workloads where bidi rendering is
not available. And I see no problem with function doing something that
is achievable by other functions.

• convert_cyr_string() -- Point to mb_convert_encoding() / iconv

Maybe just make it a pseudo-alias for iconv?

• The alternative string interpolation syntaxes (${varName},
  ${varName['offset']}, ${expr}) and make them more consistent
  ({$varName}, {$varName['offset']}, {${expr}})

I'm not sure how one is "more consistent" than the other.

• The historial parameter handling that works both ways for
  implode(), should be unified to match that of explode()

I'd advise against messing with such widely used function as implode()...

--
Stas Malyshev
smalyshev@gmail.com

Hi

Den søn. 24. jun. 2018 kl. 18.47 skrev Nikita Popov nikita.ppv@gmail.com:

If you have a minor deprecation in mind, but were too lazy to write an RFC
for it, please write me a mail until tomorrow, so that it might be included
as part of this proposal. As time is limited I don't want to include
anything larger or controversial in this RFC though.

As suggested in the past, I would like to add the following to this
list (if its not too late):

• The (real) type-cast and its function, is_real(). There doesn't
  seem to be any support for reals in settype() anyway (side note: in
  the implementation of settype() it claims "double" is deprecated)
• Function variants that already exists as constants, php_sapi_name()

PHP_SAPI, pi() > M_PI, phpversion() > PHP_VERSION etc

Keep in mind you can do phpversion("extensionname"), so that function
at least can't be removed, as the constant doesn't provide the same
functionality.

David

On Tue, Jun 26, 2018 at 10:22 PM, Kalle Sommer Nielsen kalle@php.net
wrote:

Den søn. 24. jun. 2018 kl. 18.47 skrev Nikita Popov <nikita.ppv@gmail.com

:
If you have a minor deprecation in mind, but were too lazy to write an
RFC
for it, please write me a mail until tomorrow, so that it might be
included
as part of this proposal. As time is limited I don't want to include
anything larger or controversial in this RFC though.

I took the liberty and added the FILTER_SANITIZE_MAGIC_QUOTES to the
RFC so we may once and for all get rid of magic_quotes.

After looking into this, I think that FILTER_SANITIZE_MAGIC_QUOTES may be a
legitimate filter, which just has a bad name. Next to other filters that
perform htmlspecialchars and urlencode, it makes sense that there is also a
filter that performs addslashes. Maybe we should just rename this filter to
something which is not tainted by the "magic quotes" terminology?

Nikita