If-Modified-Since is wrong by comment feed

[sweetie089]

Processing of "If-Modified-Since" is wrong by comment feed.
If time newer than the last contribution time is specified, "304 Not Modified" will certainly return.

procedure for reproducing:

• Prepare wordpress in which post exists.
• Record the present time.
• Comment.
• Make "If-Modified-Since" from the recorded time, and access "​http://DOMAIN/?feed=comments-rss2."

Result:

• "304 Not Modified" returns.

Created patch uses the last comment time for a branch condition, when "comments-rss", and "comments-rss2" and "comments-atom" are included in "feed" of the URL parameter.

Although creation of the test code was also tried, it gave up for the following reason.

• The exit function is performed within a function.
• There is no how to acquire the contents set by header.

sorry.

[SergeyBiryukov]

Confirmed the bug.

We check for $this->query_vars['withcomments'] (tags/3.5.1/wp-includes/class-wp.php#L338​), but it's only set for comment feeds if permalinks are enabled.

[sweetie089]

Thank you for the check.

Indeed, it is normal if the permalink is effective.
Let's imitate a permalink and set "withcomments" by "parse_request".

Since the remade patch is attached, please use the favorite one.

[SergeyBiryukov]

Found a similar block in WP_Query::parse_query(): tags/3.5.1/wp-includes/query.php#L1597​.

[SergeyBiryukov]

Introduced in [2384]. Related: [2627], [4483].

[wonderboymusic]

these are all marked 3.7-early

[nacin]

So it seems like that block from WP_Query actually needs to be moved entirely into the WP class. Is there even a reason to have that in WP_Query, other than for direct use of WP_Query for preparing a comments feed *without* using withcomments? (Fine with keeping it, just want to make sure we look at it from all angles.)

Because of the block in WP_Query, we should use the same logic in the WP class. If comments-*, strip it and set withcomments.

Is this easily unit testable? That would be nice.

[nacin]

Looking at this again, I actually think the logic in and location of the first patch was closer to ideal than the second patch.

WP_Query specifically sets withcomments to 1 if the feed query variable contains comments-, even if withoutcomments is set. It overrides withoutcomments, which is only consulted when withcomments is not specified and we are dealing with a singular item.

So, the logic needs to be moved up a bit, to ensure the behavior is the same.

I don't think there is any unit tests here; that would be good to have in the future.

[nacin]

In 25683:

Ensure wp::send_headers() detects a comments feed when permalinks are disabled and thus the withcomments QV is omitted. This fixes Last-Modified.

props sweetie089.
fixes #24622.