Support from Acquia helps fund testing for Drupal Acquia logo

Comments

olbion’s picture

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

  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;
  }
ParisLiakos’s picture

Thank you!
attaching a patch for this

sijuwi’s picture

The patch has a missing )

sijuwi’s picture

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?

lukus’s picture

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.

ParisLiakos’s picture

Thanks @sijuwi.
corrected patch attached

@lukus: try this patch

bgm’s picture

Status: Needs review » Fixed

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

Jānis Bebrītis’s picture

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.

Jānis Bebrītis’s picture

patch for latest dev code

mrfelton’s picture

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

mrfelton’s picture

Patch updated against dev.

mrfelton’s picture

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.

bgm’s picture

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

Jānis Bebrītis’s picture

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?

bgm’s picture

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.

mrfelton’s picture

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.

mikeytown2’s picture

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 ;)

mikeytown2’s picture

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

bgm’s picture

Status: Needs review » Reviewed & tested by the community

Good idea!

bgm’s picture

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.

Balbo’s picture

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.

madelyncruz’s picture

Issue summary: View changes

#9 works with the latest dev version. Thank you.

hoter’s picture

Version: 6.x-1.21 » 7.x-1.x-dev
FileSize
619 bytes

Hi guys,
thank you for your awesome module. #9 works fine. But what about post queries via AJAX? I have a form which submits data via AJAX and receives DRUPAL_UID=-1. And any next query (GET or POST) will be done by Drupal. I won't html file because I have DRUPAL_UID in cookies.
I guess to not send this cookie for the POST queries via AJAX. Please have a look at my patch and let me know what do you think.

smitty’s picture

I have installed Boost 7.x-1.2 and I am using acpu and opcode. I had a look at the code and the patches of this issue are really committed to this version. But my problem is: The message a user should see after registration is not shown on the frontpage. The code seems to work fine, as setcookie returns true. And it takes a while until the frontpage is rendered, so I think it's not a cached page which is delivered after registration. But the registration message is missing! This message is not displayed until I go to another page, which is not cached by boost like the contact form (containing a captcha).

As soon as I switch off Boost or tell boost not to cache the frontpage everything works as assumed again.

Contrary to the user registration the contact form and webform are working well as their messages are displayed.

So I think its a special problem of the registration form.

Can anybody confirm this behaviour? Has anybody a clue what could cause this problem?