Ticket #1323 (closed defect (bug): fixed)

Opened 7 years ago

Last modified 7 years ago

Feeds return 304 when no new posts have been made

Reported by: anonymousbugger Owned by: matt
Priority: normal Milestone:
Component: Optimization Version: 1.5.1
Severity: minor Keywords:
Cc: dougal, Waldo, peterj

Description

While checking that I hadn't broken my WP install by upgrading to 1.5.1, I discovered that all of the feeds were returning 304 for all requests, no matter if they should be returning 200 + content or not. When I posted a new entry the feeds began to behave correctly again.

Attachments

last-modified.2.patch Download (683 bytes) - added by anonymousbugger 7 years ago.
last-modified.patch Download (683 bytes) - added by anonymousbugger 7 years ago.
wp-blog-header.diff Download (1.4 KB) - added by anonymousbugger 7 years ago.

Change History

  • Patch set to No

I can confirm the same. A quick post about v1.5.1's release fixed my main feed and a quick comment post fixed my comments feed. Before that, they both gave off 304s.

Opening a post or comment for editing and quickly hitting "save" also fixes this issue.

The problem is with client_last_modified. If it is empty, wp-blog-header.php calls strtotime("") and that returns '00:00:00 of the current day'.

Adding an extra '$client_last_modified &&' fixes the problem. Diff is in the attachment.

edited on: 05-10-05 15:01

Uploaded corrected diff

Patch #2 works like a charm.

I think a better check would be to use !empty() instead of isset() when setting the variable, with a trim() for good measure:

if (!empty($_SERVERHTTP_IF_MODIFIED_SINCE?))

$client_last_modified = trim($_SERVERHTTP_IF_MODIFIED_SINCE?);

This way, we won't set the variable if the browser didn't send the header. And even if they send whitespace for the value, we'll set an empty string, which will fail the boolean tests later.

No, that's not enough. You have to fix the tests too, they do not work correctly if client_last_modified is an empty string currently.

BAStats also conflicts with the RSS feeds in v1.5.1, it's also not compatible with v1.5.1.

Patch 2 is working perfectly so far. For those who need a more English explanation, see this:  http://fernando.dubtribe.com/archives/2005/05/10/solution-to-wp151-rss-errors/

edited on: 05-11-05 17:11

Hmmm.... When passed an empty string, strtotime() is returning a timestamp for midnight on the current day rather than treating it as an invalid date and returning -1 or 0 as I would have thought. That's pretty annoying, but it is documented on the GNU page pointed to from the PHP docs. It would have been nice if the PHP docs directly documented it, though.

Okay, so that's the crux of the problem. Knowing that, we can write a better fix. We might just want to forego attempts to write the logic 'elegantly', and just put a chunk of more verbose logic there that handles the various possibilities in a more transparent fashion.

This may be a crazy question, but why not switch back to the way feeds were handled in v1.5.0? I can't recall anything wrong with them, certainly not on this level at least.

I just uploaded wp-blog-header.diff, which I think should cover the edge cases better now.

macmanx: the 1.5.0 behavior was not completely correct. It didn't return a "304 Not Modified" response if only one of the ETag or Last Modified headers were sent by the browser, only if it sent both.

Thanks, dougal!

If I understood the .diff correctly ((-) means to remove the line, (+) means to add the line), then this fix appears to at least not cause any problems. Time will tell if it works or not. Additionally, a forum user has suggested that the feeds may be breaking as soon as the next day rolls over (IOW, midnight).

edited on: 05-12-05 02:56

Yes, it's a 'unified diff: '-' means remove, '+' means add. You can apply patches with the 'patch' command.

The roll-over problem just happens with "adding a new post/comment fixes it" work-around. It's no problem with the patched code.

  • Owner changed from anonymous to matt
  • fixed_in_version set to 1.5.1.1
  • Status changed from new to closed
  • Resolution changed from 10 to 20
Note: See TracTickets for help on using tickets.