Hi,

it seems that Drupal messages set threw drupal_set_message get cached. A way to avoid it is to add || messages around line 447 in boost_is_cacheable function of boost.module file.

Comments

I think it's better to deal with it in hook_init. I added another condition to line 166:

<?php
 
if (   strpos($_SERVER['SCRIPT_FILENAME'], 'index.php') === FALSE
     
|| $_SERVER['SERVER_SOFTWARE'] === 'PHP CLI'
     
|| ($_SERVER['REQUEST_METHOD'] != 'GET' && $_SERVER['REQUEST_METHOD'] != 'HEAD')
      || isset(
$_GET['nocache'])
      || empty(
$_boost['menu_item']['status'])
      ||
$_boost['menu_item']['status'] != 200
     
|| variable_get('site_offline', 0)
      || isset(
$_SESSION['messages']) //Added to prevent caching pages that render messages
 
) {
   
$_boost['cache_this'] = FALSE;
  }
?>

StatusFileSize
new453 bytes

Thank you!
attaching a patch for this

The patch has a missing )

Also this appears to get rid of messages on a pages such as a product listing page in ubercart where a message is required each time an item is added to the cart?

Hi

I'm experiencing a problem in Drupal Commerce where the cart update message is being removed when boost is active.

Should I apply the fix mentioned here? I'm currently using the 7.x-1.x-dev published on the 23rd Dec.

StatusFileSize
new404 bytes

Thanks @sijuwi.
corrected patch attached

@lukus: try this patch

Status:Needs review» Fixed

Thanks for the patch! (and sorry it took so long to commit)
Committed to 7.x-1.x.

Status:Fixed» Needs review

this logic also needs to remove page that is being POST'ed to.

i.e.:

1. user loads page contacts
- boost saves page as "contacts_.html"
2. user posts message into contact form
- POST does not get content from cache, it's ok
- page executes drupal_set_message("subbmission ok")
3. page get's reloaded, but doesn't reach php because gets rewritten to previous contacts_.html
4. only on next POST user receives $_SESSION['message'];

I fixed it by checking if POST request received, then check if current page cache file_exists and in case it already exists, clear it. like this:
boost.module / boost_init()

   // try to clear cache for this page so that drupal_set_message() shows up on page reload
     if (   (strpos($_SERVER['SCRIPT_FILENAME'], 'index.php') == TRUE )
      && ( ($_SERVER['REQUEST_METHOD'] == 'POST') || isset($_SESSION['messages']))
     ) {
      $_boost = boost_transform_url();
      if (file_exists($_boost['filename'].'.html')) {
        unlink($_boost['filename'].'.html');
      }
      $_boost = array(); // clean variable afterwards
    }
    $_boost['cache_this'] = FALSE;

and this requires #6 patch applied too. I can provide full patch for this if I have the right idea on this problem.

StatusFileSize
new847 bytes

patch for latest dev code

With the patch in #9 applied, the caching of pages with drupal messages seems to work very well. Thanks!

StatusFileSize
new842 bytes

Patch updated against dev.

StatusFileSize
new998 bytes

Also, I believe this line is wrong in the check for things that should/shouldn't be cached:

isset($_SESSION['messages']) // do not cache pages with messages

it should use an empty() check instead, which means that there are actually no messages. Updated patch applied.

I committed the patch for the isset/empty issue.

However, I tend to disagree with flushing the page cache on POST. Flushing the cache for that page will affect all users, not just the user who has submitted the form. On a high traffic site, this will have a big impact.

Imho, it should be in the .htaccess that we should detect situations where not to serve the cached file:

  # Caching for anonymous users
  # Skip boost IF not get request OR uri has wrong dir OR cookie is set OR request came from this server OR https request
  RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [OR]
  RewriteCond %{REQUEST_URI} (^/(admin|cache|misc|modules|sites|system|openid|themes|node/add|comment/reply))|(/(edit|user|user/(login|password|register))$) [OR]
  RewriteCond %{HTTP_COOKIE} DRUPAL_UID [OR]
  RewriteCond %{ENV:REDIRECT_STATUS} 200
  RewriteRule .* - [S=3]

In the above, it states that after a POST, or when accessing node/add, comment/reply, and other pages, the cache is not served.

However, Drupal sometimes does a redirect (drupal_goto) after a post, which means that the page is served with a GET. (this is particularly true for Ubercart/commerce, where adding a product redirects to /cart ? Although /cart should always be excluded from the cache.. 6.x.1.x checked this explicitely, but 7.x-1.x does not)

If yes, we have to options:

* the nocache=1 hack,
* or set a cookie on POST which is removed after a GET (+ check for that cookie in the .htaccess, as it does for DRUPAL_UID).

In 6.x-1.x, nocache=1 was added to redirections on hook_exit(). I have to admit, however, that I'd rather set a cookie, since the way nocache was handled in 6.x-1.x was causing quite a few bugs.. (interfering with hook_exit sounds like searching for trouble).

I'd like to see cookie approach implemented, that should absolutely work.
Also, I don't like my ".html" concatenation in patch. Do You know boost function that has it as variable?

Here is a patch that sets a "nocache" cookie on POST and removes it aftewards.

To test it, the htaccess rules need to be regenerated or add:

RewriteCond %{HTTP_COOKIE} BOOST_NOCACHE [OR]

after:

RewriteCond %{HTTP_COOKIE} DRUPAL_UID [OR]

I tested it on the Drupal core contact form (note that this page should normally not be cached if it's using a captcha).

I don't find it a particularly elegant solution, but it has the advantage of:
* being easy to override by other modules (since it is set in boost_cookie_handler, which goes through drupal_alter), and also
* other modules could set the cookie if necessary (for example, if doing a "get" on a particular URL that needs to show a message to the user).

Please test/review and let me know what you think.

StatusFileSize
new903 bytes

Here is the patch from #12 again against latest dev. I'm not convinced about the cookie approach, you may be right- but I just need the other version to apply against head for my drush make file for now, so here it is.

The approach in #16 isn't a good idea. Removing a cached page creates a race condition. Setting and removing a cookie isn't super elegant but the POST destination rewriting trick that is used in 6.x doesn't cover all use cases. I would say cookies are the way forward for this; this is how varnish does it as well, with cookies. That being said patch #15 looks pretty good. The documentation for boost_cookie_handler() should reflect the fact that it now will change 2 different cookies.

The other thing to consider is, do we need 2 different cookies? I think we might be able to get away with one cookie still. We set the boost cookie to -1 on non get/head; code is set to remove it on the next request currently ;)

Only using one cookie in this patch. no htaccess changes now ;)

Status:Needs review» Reviewed & tested by the community

Good idea!

Status:Reviewed & tested by the community» Fixed

Committed to 7.x-1.x.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Version:7.x-1.x-dev» 6.x-1.21
Status:Closed (fixed)» Active

Is it possible to backport this patch for Drupal 6?
I'm experiencing the problem "No Webform confirmation message after redirection with Boost installed" and, if I understand the point, this fix should solve it.