Getting this error in my logs after upgrading to 6.x-1.17. It occurs at boost.module on line 2871 in function boost_set_cookie. Seems to be caused by the strval() in

setcookie(BOOST_COOKIE, strval($uid), $expires, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure') == '1');

which seems to have changed recently.

Is there a particular reason why you are using strval? Does dynamic type handling not take care of any conversions automatically?

Using php 5.2, if that matters.

CommentFileSizeAuthor
#3 boost-646228.patch954 bytesmikeytown2
#2 boost-637002.patch1.51 KBmikeytown2
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeytown2’s picture

Do you call boost_set_cookie() outside of the boost module? I've changed the API for this function so it now takes the user ID instead of the user object. Sounds like the user object is still being passed to this function. This has been done so the latest changes to the way boost handles cookies is a clean implementation.

/**
 * Sets a special cookie preventing authenticated users getting served pages
 * from the static page cache.
 *
 * @param $uid
 *   User ID Number
 * @param $expires
 *   Expiration time
 */
function boost_set_cookie($uid, $expires = NULL) {

I'm using strval to make the type cast explicate, the conversion can be done automatically, I just want to make this clear that setcookie() takes a string. strval has nothing to do with the error; user object being passed to this function is whats causing the error. I could put in a check that uses is_object() and if it is then set the uid via $uid = is_object($uid) ? $uid->uid : $uid at the top of the function.

mikeytown2’s picture

Status: Active » Needs review
FileSize
1.51 KB
mikeytown2’s picture

FileSize
954 bytes

This is the correct patch

lakka’s picture

Yes I was. Sorry, I didn't spot this. I wonder if it might be better if you don't allow the $user object to be passed in, but raise a clearer watchdog error instead, such as "boost_set_cookie now requires a $uid". Otherwise people like me will never fix their code!

mikeytown2’s picture

Just curious what are you using this function for? I'm trying to fix the odd login/logout bugs so if I can better understand how people use the cookie function I have a better chance of not causing pain.
http://drupal.org/node/645900#comment-2319562

lakka’s picture

The recent beta of Flag module allows flags (eg "bookmark this page") to be set for anonymous visitors. The flags are stored in the session - see http://drupal.org/node/271582 - and so can be different for each anonymous visitor. Clicking on a flag link will change the text of the flag link from "Flag this ..." to "Unflag this ...", though the URL remains the same. However, this does not work nicely with boost, since of course boost serves the same cached page to each anonymous visitor. There are a couple of ways round this:

1) Do not cache any pages with a flag/unflag link on them. But this is no use for sites which have the links on most pages.

2) When a flag is set, set the boost cookie so that that particular user sees the uncached pages from then on, and so will see pages with the "Unflag this" link:

// Implementation of hook_flag, called when a flag is set/unset
function mymodule_flag($action, $flag, $content_id, $account) {
// set the boost cookie to ensure that cached pages are not served, and
// the user sees pages with the flag set/unset as appropriate
  global $user;
  boost_set_cookie($user->uid);
}

// Implementation of hook_boost_is_cacheable().  Make sure
// that pages with a flag set are never cached
function mymodule_boost_is_cacheable($path){
  // get node id from path
  $path = drupal_get_normal_path($path);
  $args = explode('/', $path);
  if ($args[0] == 'node') {
    $nid = $args[1];
    $flag = flag_get_flag('bookmarks');
    if($flag->is_flagged($nid)){
      return FALSE;
    }
  }
  return TRUE;
}

This works for me, though it may need some refining. It does mean that the cache is disabled for all pages for any visitors who set a flag on even one page, but I am happy with that at the moment.

I suppose this sort of approach might help with anonymous shopping carts, and modules such as fivestar.

Edit: Well, this *was* working, but I've noticed that your new code now removes the cookie in boost_init if $user->uid is 0. This means that the cookie can never remain set for anonymous users, which is a great pity. There is no other sensible way to ensure that particular anonymous users can bypass the cache. Is there another way of signalling that the cookie should be removed by boost_init on the next pass, perhaps by setting the value of the cookie to something like "remove" (i.e. non-numeric)? That way, its value could be '0' if the user is anonymous, and the cookie could stay in place.

mikeytown2’s picture

Sounds like I need to make the cookie code in the init hook be a setting that can be turned on or off. Does the core cache work with the anonymous flag?

mikeytown2’s picture

Status: Needs review » Fixed

Fixed both issues, there is a new setting "Aggressive setting of the boost cookie" disable that and you should be good to go.

Status: Fixed » Closed (fixed)

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