WP-for-Church / Sermon-Manager

Sermon Manager for WordPress is the #1 plugin for churches who want to manage their sermons easily and missionally.
https://wordpress.org/plugins/sermon-manager-for-wordpress/
GNU General Public License v2.0
44 stars 34 forks source link

Wrong RFC822 date and time stamps in podcasts RSS feed #37

Closed JakeSmarter closed 7 years ago

JakeSmarter commented 7 years ago

The current podcast RSS feed implementation produces localized RFC822 date and time stamps in the body of the [pubDate]() element of the item element instead of date and time stamps in the C or POSIX locale (often effectively the en_US locale). This violates the RSS 2.0 and also RFC822 date and time specifications, and thus causes consumers of a feed, exported by systems configured to other locales than C, POSIX, or en, to error out or ignore the time stamp.

Note that although the RSS 2.0 specification speaks of RFC822 which has been superseded by RFC5322 by now, even RFC5322 does not specify localized date and time stamps (which makes perfect sense). It is pretty safe to assume that the RSS 2.0 specification by referencing any RFC also implies any latest RFC to apply as long as it is backwards compatible (which is true for RFC5322).

Interestingly, the implementation produces correct date and time stamps in the body of the lastBuildDate element regardless of the exporting system's (or process') current locale.

nikola3244 commented 7 years ago

Thanks for the report, but pubDate element generation is handled by WordPress itself. If you believe that there is a bug, submit it to official WordPress bug tracker: https://core.trac.wordpress.org/

Related line: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/feed-rss2.php#L92

JakeSmarter commented 7 years ago

Thanks for the report, but pubDate element generation is handled by WordPress itself.

This statement is partly incorrect, or only partly correct, depending on how you look at it.

It may be correct (and probably is) for the child pubDate element of the channel element, however it is not for the child pubDate element of the item element. I did take a closer look at Sermon Manager's podcast RSS feed code. The later (item case) is definitely currently handled by the Sermon Manager directly. Hence, please reopen this issue because it only pertains to the Sermon Manager.

nikola3244 commented 7 years ago

Thanks for adding additional comments.

The line of code that I have sent is from official WordPress RSS2 code. And it is used for creating pubDate element as a child of item element, as you can see the opening item tag just couple lines above it.

It is true, we have copied that line into our own RSS2 template, as you can see here. It is completely identical to official WordPress code.

So, if there is a bug in Sermon Manager code, that means that it is present in the WordPress itself.

Unfortunately, we do not have time or resources to fix core WordPress issues, so please, if you believe that there is an issue, create it in the official WordPress bug tracker. As soon as they fix it, we will update the code in Sermon Manager.

Or, if you have time and knowledge, make a pull request and we will gladly accept it. (And again, you could also make a pull request or open an issue on WordPress bug tracker as well)

JakeSmarter commented 7 years ago

Perhaps I should have added which RSS feed I am referring to. The issue refers to the podcast RSS feed of the Sermon Manager, not the sermons feed itself.

Currently, I do not have much time to dig deeper into the Sermon Manager code either but I may post a PR to fix this issue some time in the future.

nikola3244 commented 7 years ago

Yeah, I've been talking about podcast RSS feed whole time. The links in my previous comments lead all to wpfc-podcast-feed.php file. So, it's for podcasts

JakeSmarter commented 7 years ago

At first glance, the code may look identical to WordPress RSS code, however it does not behave like WordPress code. There is some sort of function redirection or aliasing going on. The function effectively responsible for generating the pubDate element's body time stamp string in the item element of Sermon Manager is wpfc_sermon_date(). I am not a PHP expert, so I am unable to identify the root cause of the issue or what to actually fix yet. But, it is definitely not working as intended. To me, https://github.com/WP-for-Church/Sermon-Manager/blob/master/includes/podcast-functions.php#L24 looks like where this sort of function alias is going off the rails (so to speak).

Test, test, and once again test. Do not assume anything unless you have looked through the full calling stack. :wink:

nikola3244 commented 7 years ago

I took another look at this issue, and I still stay by my words - this is an issue in WordPress itself, and I will now provide proof for my statement.

First, with default settings, Sermon Manager uses core WordPress file, not custom one for Sermon dates, pubDate of item element.

The code is located at line 92 of feed-rss2.php. Copied here for consistency:

<pubDate><?php echo mysql2date('D, d M Y H:i:s +0000', get_post_time('Y-m-d H:i:s', true), false); ?></pubDate>

There is no way to override that line because of the way RSS feed is created (but actually, I got an idea but it's a bit hacky - so it won't be implemented).

If we take a look at mysql2date function:

function mysql2date( $format, $date, $translate = true ) {
    if ( empty( $date ) )
        return false;

    if ( 'G' == $format )
        return strtotime( $date . ' +0000' );

    $i = strtotime( $date );

    if ( 'U' == $format )
        return $i;

    if ( $translate )
        return date_i18n( $format, $i );
    else
        return date( $format, $i );
}

...we would see that it uses PHP's core date() function (because $translate is set to false), and PHP manual for that function says that r parameter should be used for date format if we want RFC2822 formatted date.

You said:

Interestingly, the implementation produces correct date and time stamps in the body of the lastBuildDate element regardless of the exporting system's (or process') current locale.

And I agree, because in the same file, couple lines above, WordPress uses r as the format parameter for the date. That's why lastBuildDate shows correctly.

So, that's why I strongly recommend you to submit a bug to WordPress, because this is not an issue on our end.

I would love to be proved wrong, but everything points to WordPress core files. (if you edit feed-rss2.php on your local instance and reload Sermon Manager podcasts feed, it will show changes. Don't forget to clear local browser and server cache)