Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:102462
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAESVnVrJo8BH=gMf07qq9MsgT6ziqWJO6JPgxVgVuStr+-5cpw@mail.gmail.com>
References: <CAF+90c_+GZNrV1SC9q=Y3C=L3LMnoXX7J3zK6=kS9-sMXHZZow@mail.gmail.com>
 <CAESVnVrJo8BH=gMf07qq9MsgT6ziqWJO6JPgxVgVuStr+-5cpw@mail.gmail.com>
Date: Tue, 26 Jun 2018 20:54:16 +0200
Message-ID: <CAF+90c_rhUaV1RrL4YnWLM-p9__j03EQkf8X7A6yWJVdG6oX7A@mail.gmail.com>
To: Sara Golemon <pollita@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000006864ba056f900655"
Subject: Re: [PHP-DEV] [RFC] Deprecations for PHP 7.3
From: nikita.ppv@gmail.com (Nikita Popov)

--0000000000006864ba056f900655
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 25, 2018 at 9:32 PM, Sara Golemon <pollita@php.net> wrote:

> 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. =F0=9F=
=A4=B7
>
> > 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. =F0=9F=98=95
>
> > fgetss() function and string.strip_tags filter
> >
> "...these functions seem to be of very little utility..." [citation neede=
d]
> 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.
> =F0=9F=98=A4
>

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.... =F0=9F=
=98=AC
>

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

--0000000000006864ba056f900655--