On Mon, Jun 25, 2018 at 9:32 PM, Sara Golemon <poll...@php.net> wrote:
> On Sun, Jun 24, 2018 at 11:47 AM, Nikita Popov <nikita....@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
