WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 17 months ago

#25358 new defect (bug)

Last-Modified header is ' GMT' for feeds on sites with no approved comments when using a static Posts page

Reported by: joeybvi Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.8
Component: Feeds Keywords:
Focuses: Cc:

Description

The code in WP::send_headers() (source:/trunk/src/wp-includes/class-wp.php#L339), seen below, sets the Last-Modified header for feeds. The problem is that get_lastcommentmodified('GMT') will return null if the feed is a Static Posts page (there are no approved comments, and then the Last-Modified header will be set to ' GMT'. This breaks the Etag header as well, since that will be set to md5(' GMT'). And it also breaks the sending of a 304 status. For an example, see the headers for http://keyboardheadbashing.com/blog/feed/

This also happens for a blog post's feed that has no current comments; see the headers for http://keyboardheadbashing.com/uncategorized/coming-soon/feed/

My coworker also just pointed out that if there is an older comment (say, a comment from 11/11/2011) but there are newer posts (say, a post from 12/12/2012), the date for the older comment will be used on both the Statics Posts page feed AND the specific-post feed (even if the post the feed is for is newer than the comment!).

			// We're showing a feed, so WP is indeed the only thing that last changed
			if ( !empty($this->query_vars['withcomments'])
				|| ( empty($this->query_vars['withoutcomments'])
					&& ( !empty($this->query_vars['p'])
						|| !empty($this->query_vars['name'])
						|| !empty($this->query_vars['page_id'])
						|| !empty($this->query_vars['pagename'])
						|| !empty($this->query_vars['attachment'])
						|| !empty($this->query_vars['attachment_id'])
					)
				)
			)
				$wp_last_modified = mysql2date('D, d M Y H:i:s', get_lastcommentmodified('GMT'), 0).' GMT';
			else
				$wp_last_modified = mysql2date('D, d M Y H:i:s', get_lastpostmodified('GMT'), 0).' GMT';
			$wp_etag = '"' . md5($wp_last_modified) . '"';
			$headers['Last-Modified'] = $wp_last_modified;
			$headers['ETag'] = $wp_etag;

The code below will use the comment timestamp if it is set AND is newer than the post's timestamp; otherwise, it will use the post's timestamp:

			// We're showing a feed, so WP is indeed the only thing that last changed
			if ( !empty($this->query_vars['withcomments'])
				|| ( empty($this->query_vars['withoutcomments'])
					&& ( !empty($this->query_vars['p'])
						|| !empty($this->query_vars['name'])
						|| !empty($this->query_vars['page_id'])
						|| !empty($this->query_vars['pagename'])
						|| !empty($this->query_vars['attachment'])
						|| !empty($this->query_vars['attachment_id'])
					)
				)
			)
				$comment_last_modified = strtotime(get_lastcommentmodified('GMT'));
			$post_last_modified = strtotime(get_lastpostmodified('GMT'));
			$wp_last_modified = date('D, d M Y H:i:s', isset($comment_last_modified) && $comment_last_modified > $post_last_modified ? $comment_last_modified : $post_last_modified).' GMT';
			$wp_etag = '"' . md5($wp_last_modified) . '"';
			$headers['Last-Modified'] = $wp_last_modified;
			$headers['ETag'] = $wp_etag;

I will attach a patch for this change.

Attachments (1)

25358.patch (856 bytes) - added by joeybvi 21 months ago.
Patch

Download all attachments as: .zip

Change History (3)

@joeybvi21 months ago

Patch

comment:1 @nacin20 months ago

There are two separate issues here. Obviously, this code is pretty old and the Last-Modified times are not necessarily reliable (which isn't great).

I think the best initial fix would be to not print a Last-Modified header when we have no comments (but when that's the data point we're using). Or, to use the last modified time for posts in this case, which probably makes more sense.

I'm not sure we should call get_lastpostmodified() always, though, without fixing #15499.

comment:2 @brokentone17 months ago

  • Cc kenton.jacobsen@… added
Note: See TracTickets for help on using tickets.