Make WordPress Core

Opened 11 years ago

Closed 7 years ago

Last modified 4 years ago

#25358 closed defect (bug) (duplicate)

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

Reported by: joeybvi's profile joeybvi Owned by: stevenkword's profile stevenkword
Milestone: Priority: normal
Severity: normal Version: 3.8
Component: Feeds Keywords: reporter-feedback
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 11 years ago.
Patch

Download all attachments as: .zip

Change History (8)

@joeybvi
11 years ago

Patch

#1 @nacin
10 years 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.

#2 @brokentone
10 years ago

  • Cc kenton.jacobsen@… added

#3 @stevenkword
9 years ago

  • Keywords reporter-feedback added

@joeybvi -- I've been unable to reproduce this in my local environments. I've tested both with 4.2.4 and 4.3 RC2.

However, I am able to see the 'GMT' value coming back in the headers at http://keyboardheadbashing.com/blog/feed/

What steps can I take to reproduce? I've been trying this and coming up empty:

  1. Create New Post without comments
  2. Navigate to /new-post/feed/
  3. Inspect request headers

#4 @stevenkword
9 years ago

  • Owner set to stevenkword
  • Status changed from new to assigned

#5 @stevenkword
7 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in 38925

#6 @swissspidy
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution changed from fixed to duplicate

Duplicate of #38027.

#7 @hasanafaque9
4 years ago

There are two separate issues here. Obviously, this code is very old, and the time of last modification is not necessarily reliable (not very good).
http://hasanafaque.com/
What steps can I take to copy? I have been trying this and left it empty:
Create a new post without comments
Navigate to /new-post/feed/
Check request header

Note: See TracTickets for help on using tickets.