Ticket #1323 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

Feeds return 304 when no new posts have been made

Reported by: anonymousbugger Assigned to: 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 (0.7 kB) - added by anonymousbugger on 05/21/05 06:38:44.
last-modified.patch (0.7 kB) - added by anonymousbugger on 05/21/05 06:38:44.
wp-blog-header.diff (1.4 kB) - added by anonymousbugger on 05/21/05 06:38:44.

Change History

05/10/05 00:07:47 changed by anonymousbugger

  • Patch set to No.

05/10/05 01:40:02 changed by macmanx

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.

05/10/05 02:34:02 changed by macmanx

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

05/10/05 14:57:47 changed by anonymousbugger

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

05/10/05 15:03:54 changed by anonymousbugger

Uploaded corrected diff

05/10/05 17:10:21 changed by peterj

Patch #2 works like a charm.

05/10/05 21:10:24 changed by dougal

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.

05/10/05 21:22:01 changed by anonymousbugger

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.

05/11/05 04:17:43 changed by macmanx

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

05/11/05 17:10:40 changed by macmanx

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

05/11/05 20:03:46 changed by dougal

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.

05/11/05 20:16:56 changed by macmanx

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.

05/11/05 20:35:46 changed by dougal

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.

05/11/05 20:53:55 changed by anonymousbugger

Thanks, dougal!

05/12/05 02:44:16 changed by macmanx

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

05/12/05 08:31:16 changed by anonymousbugger

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.

05/13/05 21:07:06 changed by matt

  • 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.

05/21/05 06:38:44 changed by anonymousbugger

  • attachment wp-blog-header.diff added.