[wp-trac] [WordPress Trac] #37082: Remove (most) uses of create_function() from core

[WordPress Trac]

#37082: Remove (most) uses of create_function() from core
-------------------------+--------------------
 Reporter:  sgolemon     |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  4.6
Component:  General      |     Version:  trunk
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |     Focuses:
-------------------------+--------------------

Comment (by sgolemon):

 > I think 0004-Remove-unnecessary-use-of-create_function-in-AtomFee.patch​
 should be fine just to remove the create_function calls.

 Right, in terms of addressing the issue of `create_function()` usage
 (which should be avoided wherever possible), these patches are focused on
 JUST that change (I subscribe to the one-diff-one-change rule).  All my
 other comments about 0004 were related to "things that should probably be
 addressed in followup diffs at some point".

 > (Also, the filter extension can be disabled, so we can't use filter_var
 anyway.)

 Ugh, fair point.  Still, for what that default implementation is trying to
 do, a simple str_replace will go a long way towards improving things.


 {{{#!php
 public static function defaultMapAttrsFunc($k, $v) {
   $k = preg_replace('/[^0-9A-Za-z_-]+/', "", $k);
   if (function_exists('filter_var')) {
     $v = filter_var($v, FILTER_SANITIZE_ENCODED);
   } else {
     // Absolute minimal safeguard, there are known browser bugs,
     // especially in old versions of IE which render this ineffective,
     // but I cringe at doing nothing at all.
    $v = str_replace('"', '%22', $v);
   }
   return "$k=\"$v\"";
 }
 }}}

 But again, something for a followup and perhaps not even necessary to fix
 depending on how the API is used by the modules which depend on it.  Just
 something I thought I'd mention while I was in the neighborhood.

 > Translations::make_plural_form_function(), which looks absolutely crazy
 to me.

 Yep, that was the usage I mentioned in the OP, and it's clearly just a
 disguised `eval()`.  It's complex enough that any work done on it deserves
 its own ticket, and certainly needs someone who understands WP's internals
 better than I.

 BTW, for context, these `create_function()` uses came up during a
 hackathon at Facebook months ago (looking for opportunities to improve
 HHVM's ability to run WP).  I informally handed these fixes off to a
 coworker who had closer ties to WP, but I guess he never got around to
 passing them on, so I recreated the (pretty simple) diffs.
 Unsurprisingly, these made no practical difference to performance, but the
 avoidance of `create_function()` was worth it in terms of correctness.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/37082#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform