Make WordPress Core

Opened 19 years ago

Closed 19 years ago

#1323 closed defect (bug) (fixed)

Feeds return 304 when no new posts have been made

Reported by: anonymousbugger's profile anonymousbugger Owned by: matt's profile matt
Milestone: Priority: normal
Severity: minor Version: 1.5.1
Component: Optimization Keywords:
Focuses: Cc:

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 (3)

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

Download all attachments as: .zip

Change History (18)

#1 @anonymousbugger
19 years ago

  • Patch set to No

#2 @macmanx
19 years ago

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.

#3 @macmanx
19 years ago

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

#4 @anonymousbugger
19 years ago

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

#5 @anonymousbugger
19 years ago

Uploaded corrected diff

#6 @peterj
19 years ago

Patch #2 works like a charm.

#7 @dougal
19 years ago

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.

#8 @anonymousbugger
19 years ago

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.

#9 @macmanx
19 years ago

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

#10 @macmanx
19 years ago

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

#11 @dougal
19 years ago

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.

#12 @macmanx
19 years ago

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.

#13 @dougal
19 years ago

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.

#14 @anonymousbugger
19 years ago

Thanks, dougal!

#15 @macmanx
19 years ago

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

#16 @anonymousbugger
19 years ago

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.

#17 @matt
19 years ago

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