[wp-trac] [WordPress Trac] #50159: Simplepie 1.5.5 - code review and modifications - fix SimplePie cache bug

[WordPress Trac]

#50159: Simplepie 1.5.5 - code review and modifications - fix SimplePie cache bug
--------------------------+------------------------------
 Reporter:  arena         |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Feeds         |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:
--------------------------+------------------------------

Comment (by afragen):

 Replying to [comment:27 arena]:
 > ok let's get back to the patch and i explain it step by step
 >
 > **WP_Feed_Cache_Transient** implements SimplePie_Cache_Base
 > implementing the interface is a requirement of simplepie
 >
 > * md5 or not md5 ... i do not know how simplepie can apply a second md5
 on a part of a string ... still a mystery for me. Anyway, if you want to
 remove this md5, just do it !

 In https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-
 simplepie.php#L575 the default `$cache_name_function = 'md5'`. As you can
 see in the source there is a method `set_cache_name_function()` that
 allows the user to select an alternate hashing method. The point  is that
 it's already being hashed. You don't need to do that in your patch.

 The question is why did you change `$this->name` from `$location` to
 `$location:$extension`. This will only cause problems and serves no
 function other than change.

 > * transient lifetime is passed as a parameter of simplepie cache
 location (to avoid to have the same filter called twice, meaning not
 executing twice the same add_filter code if any

 There are many filters that are called more than once in different
 locations. There is no point in removing an instance of it being called as
 there is no harm in being called. It's not like removing a call to
 `add_filter` will provide a performance benefit.

 > **WP_Feed_Cache**
 >
 > see simplepie doc
 >
 > **feed.php**
 >
 >
 > * ''$feed->registry->register'' the new way to set class in SimplePie

 Other than creating a new way to set a class in SimplePie, what's the
 benefit?

 You also state that you are fixing a bug. What is the bug??

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