Drupal messages (status/error) are cached along with nodes

Alas, after following your instructions carefully, I can't reproduce this bug.
Even with aggressive caching turned on things work smoothly (reloading home page makes message disappear).
It looks like a browser+server+other_unguessable_variables issue to me. Since things work as expected in the vast majority of the cases, I'd look into some $_SESSION handling issues (?). You may try to var_dump($_SESSION) at the end of each page load and see what goes? But this wouldn't explain the cross-browser part of the issue...

I am running PHP 5.2.1 and Apache 2.2.3 Multi-Processing Module (MPM) (in ubuntu).

Unreproducible bugs are weird, anyway.

I can confirm the bug. My forum post can be found here.

In my post, I mention that I cannot reproduce the bug on my localhost (possibly because it cannot send e-mails), but I can reliably re-create it on my web host.

This would seemingly be a critical bug, because cache is, in essence, broken.

It may only be manifesting itself when the front page is a node, rather than a module-generated pages.

I've got this bug in clean instalation of Drupal 6.0-rc4. Here is an issue http://drupal.org/node/219652

http://drupal.org/node/219652 was duplicate. See also this forum report: http://drupal.org/node/205521

Moving to 6.x - bugs get fixed in the development release then back ported.

#6 works for me in Drupal 6.0-rc4. Great! I've just also tested it on Drupal 5.x with Simplenews and it works too. Very good news!

Then it's RTBC.

@chx - can you explain whats wrong with the current code that it causes thus bug? your new code looks good but i am missing the "why".

I suspect some php goof here , we basically used ob_get_contents as our gatekeeper which apparently fails. I instead pass a flag "yes i did start ob!" and then recounting messages is not needed at all. Even the request method is not absolutely necessary but I left it in as a sneaky hacky way to stop page set cache for whatever reason.

page_get_cache() already checks for uid and the GET method, so this looks like some code duplication there. (If not for micro optimized speed, to avoid a function call, then I don't see why to do it). Even if the page was cached, page_get_cache() should return FALSE under these conditions with uid != 0 or !GET.

Here is an updated patch with some phpdoc improvements as well. Please test.

#12 works good in my Drupal 6.x and 5.x instalations .

Hm, I left in the user uid checking because you might get logged in during the page run and I left in the request method caching as I stated above so that there is a sneaky way of disabling page set cache just in case. Not a terrible code duplication, I say.

Indeed, the user might have been logged in since the request started. OK, brought these two conditionals back. Thanks kndr for retesting. Committed the attached one. RTBC for 7.x as well.

How would you sneakily disable it - by setting REQUEST_METHOD to something other than GET manually?

Also, is your complaint with #12 that the status gets stored statically and checked only once?

Cross-post.

Exactly, by setting REQUEST_METHOD. It is a hack, but how often do we need page_set_cache disabled? Extremely rarely so we do not need a full fledged API to disable it but leaving a catdoor open is good.

Ouch! Did you check cache_page table? There is no records, no cached pages! Sorry. I did not check this. It looks that cache is turned off by this patch. Am I wrong?

Do you mean {cache_page}? I will test to see if I can reproduce this.

Reproduced, alas. Page cache mode is set to normal, I am browsing as an anonymous user, and try as I might I cannot get {cache_page} to gain any records.

Edit: Aggressive caching with minimum lifetime of one day makes no difference either. I will add some debugging output to trace this issue...

Odd. Adding a var_dump call to the function has produced bool(true) at the start and end of the page (expected), and also I now have a record in {cache_page}.

I hate it when this happens. Trying to remove the var_dump again to see if this really does it. If it does, I am stumped.

Edit: s/page_cache/cache_page

Well, good news (good?) and bad news:

1.) {cache_page} is indeed only filled with the following code:

function page_get_cache($status_only = FALSE) {
  static $status = FALSE;
  global $user, $base_root;

  $cache = NULL;

  if (!$user->uid && $_SERVER['REQUEST_METHOD'] == 'GET' && count(drupal_set_message()) == 0) {
    $cache = cache_get($base_root . request_uri(), 'cache_page');

    if (empty($cache)) {
      ob_start();
      $status = TRUE;
    }
  }

  echo "test"; // the {cache_page} table will only be filled if this line is here.

  return $status_only ? $status : $cache;
}

2.) {cache_page} will contain "test". Nothing else.

Perhaps we're inadvertently closing the output buffer somewhere before output is actually started?

Edit: s/page_cache/cache_page

Edit2:

Might this do it?

function page_get_cache($status_only = FALSE) {
  static $status = FALSE;
  global $user, $base_root;

  $cache = NULL;

  if (!$user->uid && $_SERVER['REQUEST_METHOD'] == 'GET' && count(drupal_set_message()) == 0) {
    $cache = cache_get($base_root . request_uri(), 'cache_page');

    if (!$status && empty($cache)) { // only start the buffer once.
      ob_start();
      $status = TRUE;
    }
  }

  return $status_only ? $status : $cache;
}

Or this?

function page_get_cache($status_only = FALSE) {
  static $status = FALSE;
  global $user, $base_root;

  $cache = NULL;

  if (!$user->uid && $_SERVER['REQUEST_METHOD'] == 'GET' && count(drupal_set_message()) == 0) {
    $cache = cache_get($base_root . request_uri(), 'cache_page');

    if (!$status_only && empty($cache)) { // only start the buffer if we are not just returning the status.
      ob_start();
      $status = TRUE;
    }
  }

  return $status_only ? $status : $cache;
}

I got this weird suspicion we may be starting the buffer twice.

I've solved the problem by using !$status as in the second of the three code blocks above. page cache now works as advertised.

I don't know the policy here. Do we have to roll-back and re-roll the complete patch, or is it enough to tack on this new fix in a separate patch?

Here it is, anyway.

looks like a cross post to me.

(This is a cross-post too, but luckily I got the email alert and can keep the status updated in this form...)

I guess either of these fixes work. Just to be safe, however, why not check the $status as well (as mine does)? If we're already paranoid about ob and setting our own flag, we might as well prevent ourselves from starting ob more than once by actually using the flag there.

chx's patch was more in line with what we would do. The $status_only flag is just a parameter to ask for the status recorded statically from the beginning of the page request, so no need to go into the other parts of the function, and waste our time there (additionally to causing bugs by opening new output buffers). Arancaytar, thanks for the analysis, but I think chx's patch is better this time.

Committed his patch to 6.x and attaching a combined patch which was rolled against HEAD. This one is to be committed to 7.x.

Never got applied to D7 by the looks of things (and still applies).

my post below has been fixed. my problem was not related to caching. FYI, it was due to debug output from a hook in using multiping:

http://drupal.org/node/245554

++++++++

Does this problem manifest as messages such as

op=load
op=view
op=alter

appearing in a box (green) above the node??

I'm seeing this in a clean install of 6.2 with caching enabled.

Any chance of getting this applied to 5.x as well?

Michelle

Hi

this issue has not been responded to as far as I could find.

The only fast way to stop this appearing for me was commenting out / hacking the template.

<!--    <?php if ($messages) : print '<div class="messages">' .$messages . '</div>'; endif; ?> -->

Pretty poor solution, but it does the job.

Seems that V6 is just not ready for "simple" public use.

Cheers

Roland

I've implemented javascript side messages setting and getting in my js_theming project.

This approach could be leveraged to solve the problem we are having. i dont know how the end result will be reached but this could be the first step to invoke messages through javascript rather than printing them via php.

more information at: http://drupal.org/project/js_theming

Still applies to 7.x with offset.

Probably doesn't apply anymore now that #231190: Page Cache doesn't work with HEAD requests gone in.

Very true. This is a straight re-roll of #28.

I suggest the following plan: this can go in for D7 as is, but we'll need a test for it.

IMO we shouldn't hold up small yet critical/out of sync bug fixes due to tests getting written - at least not until core has zero known test failures. Testing uptake is going to be slow until we can demonstrate that failed tests are down to people's own code rather than a buggy testing framework or buggy tests themselves.

For now I've opened a new issue for the tests at #284271: Tests for drupal_set_messages are cached for anonymous users and marked this back to needs review. Others may disagree of course, so feel free to change it back if I've not won you over ;)

Fine by me.

Patch #36 still applies.

See: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476

All tests have been passing for months, so we need one for this issue now!

Test along with alternate solution provided in #54238: page cache might show messages.

Something roughly similar to the latest patch was committed to HEAD a few days ago as part of #201122: Drupal should support disabling anonymous sessions. Does that fix this issue?

Hmm...I'm not sure it does. I'll have to run the tests from #54238: page cache might show messages to see if they actually did fix it.

Messages are still in fact cached for anonymous users on HEAD.

tagging

That should not happen anymore in D7. Does it apply to D6?

Setting version + status since it should be OK for D7 (certainly I've not seen any problems).

I had same problem, but i fixed checking my user table and I found that mysql client change uid = 0 (anonymous) to 11 (or something). I change to 0 again and it work like a charm.

I'm having this issue in Drupal 6.19 with pagecache turned on. Has the issue been fixed and then re-entered at a later version?
Roald

Is this still an active issue?

Demoting this due to inactivity for over a year.