podlove / podlove-publisher

Podlove Podcast Publisher for WordPress
https://wordpress.org/plugins/podlove-podcasting-plugin-for-wordpress/
MIT License
299 stars 84 forks source link

Possible bug in get_the_excerpt filter #1372

Closed retrorism closed 1 year ago

retrorism commented 1 year ago

Hi everyone! First of all: thank you for creating this plugin, we rely on it on one of our largest projects and it's been great to have this solution available. We stumbled upon a bug on this project, which we traced back to a hook definition found in this plugin:

Expected behavior

When rendering an excerpt for a post, I would think that there should be no filters that interfere with the retrieval of the post's excerpt. To ensure that this is the case, all filters that hook into get_the_excerpt need to use the hook's (optional) argument to pass the post object. Using the global $post object can cause side effects.

Actual behavior

In the method definition below, the global $post is used to retrieve the post object: https://github.com/podlove/podlove-publisher/blob/4557a2db8f5b0c2a1d0273f5a82c7a625cfa601b/lib/podcast_post_type.php#L132-L144

Instead, it should source the object from the $post parameter. In this method, the hook argument is called $excerpt. This is probably because you would expect that a filter takes in the excerpt string to return the filtered excerpt (I would make the same assumption). I'm not sure if simply switching away from the global $post object would have unexpected side effects for others, though! One way to ensure backwards compatibility would be to compare the global post object with the one passed, and if they are different posts, favour the one passed as an argument -- or do nothing.

System information (see Podlove > Support menu)


Not relevant
eteubert commented 1 year ago

Good catch! I think it's fine to take the simple fix as @avovkdesign proposed.

I took a minute for some digging and came back with a chuckle: The $post argument to the filter came with WordPress 4.5.0, April 2016. I wrote the first version of the filter in October 2012 (https://github.com/podlove/podlove-publisher/commit/5a134a8baaec1525dabf910e1ce607e1b97e41b0), years before the second parameter was added. Now I feel old 😅

retrorism commented 1 year ago

Thanks Eric! And yeah hee hee, I can relate, my first WP stuff dates 15 years back (nothing as elaborate or cool as Podlove though!) @avovkdesign and I were able to find a workaround in the interim, but with this merged, we can soon update and get rid of that workaround. Funny question perhaps: do you happen to know a Ruotger (Roddi)?

eteubert commented 1 year ago

I talked to him a couple times, not sure if that counts as “knowing” him ;)