Awhile back I noticed that the node_page_title() function in node.module was frequently producing run-time warnings because its argument wasn't an object.

I submitted a work-around patch which got rejected. I was told to find and fix the offending code that is calling the function incorrectly.

In my installation, the usual offender is buried within the _menu_item_localize() function of the includes/menu.inc file. Specifically, at around line 503, is the following line of code:

   $item['title'] = call_user_func_array($callback, menu_unserialize($item['title_arguments'], $map));

For at least some of my menu options, $callback is equal to "node_page_title". When that occurs, the code is incorrect for at least the reason that menu_unserialize() always returns an array instead of the object that node_page_title() expects.

Worse, the array it returns (at least on my installation) is something like Array ([0] => 29)) which, even if it were converted to an object, obviously doesn't contain the page title. The unserialized version contained in the database is always a:1:{i:0;i:1;} which apparently is a default value.

For the most recent traceback in my logs, the relevant call chain is:

  • _menu_link_translate() calls _menu_item_localize($item, $map, $link_translate) with
    $item['title_callback'] set to 'node_page_title'
    $item['title_arguments'] set to 'a:1:{i:0;i:1;}'
    $map set to Array ([0] => 'node', [1] => 29)
    $link_translate set to 1
  • _menu_item_localize() calls node_page_title($node) with
    $node set to Array ([0] => 29)
  • node_page_title() throws an error
    because its only line of code assumes that $node->title exists.

I'm sorry that I don't have a fix for the problem. For now, on my site, I'm going to work around the problem as follows:

/**
 * Title callback.
 */
function node_page_title($node) {
  if (!(is_object($node))) {
    if (is_array($node)) {
      if (is_integer($node[0])) {
        $node = node_load($node[0]);
      }
      else {
        $node = node_load($node);
      }
    }
    elseif (is_integer($node)) {
      $node = node_load($node);
    }
    else {
      return NULL;
    }
  }
  return $node->title;
}

Please feel free to find and implement a simpler, more rational fix.

Comments

Anonymous’s picture

Anonymous (not verified) commented

One of the reasons why typecasting the parameters would be very good. But to resolve the issue of using the result of menu_unserialize() as a parameter you can typecast the result with (object)menu_unserialize() and the array parameters will become object parameters.

I disagree with the mentality of

I was told to find and fix the offending code that is calling the function incorrectly.

and would much rather have the function protect itself from misuse. It is too bad that this issue will be carried into D7.

pillarsdotnet’s picture

pillarsdotnet commented

Actually, typcasting to an object still wouldn't solve the problem, as the array doesn't have a key called 'title'.

damien tournoud’s picture

damien tournoud commented
Status: Active » Postponed (maintainer needs more info)

That means that some module in your installation is trying to do something apparently silly. I believe that node_page_title() is *not* called with array(29), but simply with the integer 29, which basically means that node_load(29) is not called, or that something misbehaves somewhere in the process.

Could you try to dump the full $item which makes the error appear?

damien tournoud’s picture

damien tournoud commented

@pillarsdotnet: any news about that? have you been able to dump the full $item?

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Postponed (maintainer needs more info)

Sorry; haven't checked status in a couple of days.

If you look at the code of the node_page_title() function, it expects to be passed an object, not an integer or an array.

I can give you the backtrace from which I inferred the original report. In that particular case, the offending menu item is a link to a book page.

  1. Assertion failed in modules/node/node.module:1589
  2. node_page_title(29)
  3. call_user_func_array(node_page_title, Array (0 => 29))
    called at includes/menu.inc:503
  4. _menu_item_localize(
  Array (
    load_functions => a:1:{i:1;s:9:"node_load";},
    to_arg_functions => ,
    access_callback => node_access,
    access_arguments => a:2:{i:0;s:4:"view";i:1;i:1;},
    page_callback => node_page_view,
    page_arguments => a:1:{i:0;i:1;},
    title => SimExodus,
    title_callback => node_page_title,
    title_arguments => a:1:{i:0;i:1;},
    type => 4,description => ,
    menu_name => primary-links,
    mlid => 611,
    plid => 0,
    link_path => node/29,
    router_path => node/%,
    link_title => SimExodus,
    options => Array (
      attributes => Array (
        title =>
      )
    ),
    module => menu,
    hidden => 0,
    external => 0,
    has_children => 0,
    expanded => 0,
    weight => 0,
    depth => 1,
    customized => 1,
    p1 => 611,
    p2 => 0,
    p3 => 0,
    p4 => 0,
    p5 => 0,
    p6 => 0,
    p7 => 0,
    p8 => 0,
    p9 => 0,
    updated => 0,
    in_active_trail => ,
    access => 1,
    href => node/29,
    localized_options => Array (
      attributes => Array (
        title => 
      )
    )
  ),
  Array (
    0 => node,
    1 => 29
  ),
  1
)

    called at includes/menu.inc:661

  5. _menu_link_translate(
  Array (
    load_functions => a:1:{i:1;s:9:"node_load";},
    to_arg_functions => ,
    access_callback => node_access,
    access_arguments => a:2:{i:0;s:4:"view";i:1;i:1;},
    page_callback => node_page_view,
    page_arguments => a:1:{i:0;i:1;},
    title => SimExodus,
    title_callback => node_page_title,
    title_arguments => a:1:{i:0;i:1;},
    type => 4,
    description => ,
    menu_name => primary-links,
    mlid => 611,
    plid => 0,
    link_path => node/29,
    router_path => node/%,
    link_title => SimExodus,
    options => Array (
      attributes => Array (
        title => 
      )
    ),
    module => menu,
    hidden => 0,
    external => 0,
    has_children => 0,
    expanded => 0,
    weight => 0,
    depth => 1,
    customized => 1,
    p1 => 611,
    p2 => 0,
    p3 => 0,
    p4 => 0,
    p5 => 0,
    p6 => 0,
    p7 => 0,
    p8 => 0,
    p9 => 0,
    updated => 0,
    in_active_trail => ,
    access => 1,
    href => node/29,
    localized_options => Array (
      attributes => Array (
        title => 
      )
    )
  )
)

    called at includes/menu.inc:1009

  6. _menu_tree_check_access(
  Array (

    (...skipping other menu items for brevity...)

        611 => Array (
      link => Array (
        load_functions => a:1:{i:1;s:9:"node_load";},
        to_arg_functions => ,
        access_callback => node_access,
        access_arguments => a:2:{i:0;s:4:"view";i:1;i:1;}
        page_callback => node_page_view,
        page_arguments => a:1:{i:0;i:1;},
        title => SimExodus,
        title_callback => node_page_title,
        title_arguments => a:1:{i:0;i:1;},
        type => 4,
        description => ,
        menu_name => primary-links,
        mlid => 611,
        plid => 0,
        link_path => node/29,
        router_path => node/%,
        link_title => SimExodus,
        options => Array (
          attributes => Array (
            title => 
          )
        ),
        module => menu,
        hidden => 0,
        external => 0,
        has_children => 0,
        expanded => 0,
        weight => 0,
        depth => 1,
        customized => 1,
        p1 => 611,
        p2 => 0,
        p3 => 0,
        p4 => 0,
        p5 => 0,
        p6 => 0,
        p7 => 0,
        p8 => 0,
        p9 => 0,
        updated => 0,
        in_active_trail => ,
        access => 1,
        href => node/29,
        localized_options => Array (
          attributes => Array (
            title => 
          )
        )
      ),
      below => 
    ),

    (...skipping other menu items for brevity...)

      )
)

    called at includes/menu.inc:947

  7. menu_tree_page_data(primary-links)
    called at includes/menu.inc:1252
  8. menu_navigation_links(primary-links)
    called at includes/menu.inc:1217
  9. menu_primary_links()
    called at includes/theme.inc:1809
pillarsdotnet’s picture

pillarsdotnet commented
Status: Postponed (maintainer needs more info) » Active
pillarsdotnet’s picture

pillarsdotnet commented
Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

Well, I'm not using d6 anymore, and it appears that nobody else has experienced this problem, so I'm going to close the issue.

dman’s picture

dman commented

For searchers that may find this issue: This issue is the best analysis of the actual problem ... though the root cause that gets us here is still undefined.
I also agree that Drupal core should include a lot more assertions, input checking and waterproofing against unexpected input. But I can't convince anyone of that either.

Anyway - I HAVE encountered this in D7, though replicating it is indescribable - I was called in to pick up a site with 120 experimental modules turned on and off so there is no audit trail.

After disabling all the suspects I could find mentioned in other threads, including
search by page, #1709914: Notice: Trying to get property of non-object in node_page_title() (Zeile 2108 von /var/www/modules/node/node.module).
menu_block #1318210: "Trying to get property of non-object in node_page_title" error when cron runs from page view trigger.
page title #961778: Trying to get property of non-object in node_page_title_pattern_alter()
ds_extras, and taxonomy_menu
It still didn't go away.

Eventually I found some dud data in my menu table:

mysql> select menu_name, count(*) from menu_links group by menu_name;
+--------------------+----------+
| menu_name          | count(*) |
+--------------------+----------+
| 0                  |       55 |
| Array              |        5 |
| book-toc-150       |        2 |
| devel              |       15 |
| main-menu          |        7 |
| management         |      890 |
| menu-site-sections |      134 |
| navigation         |      204 |
| shortcut-set-1     |       10 |
| user-menu          |        5 |
+--------------------+----------+

- the menu_name entries of "0", and "Array" are screwy and I dunno how they got there. Somebody was playing with dev modules I think.
Looking at them, I confirmed that the entries were reflecting the pages this error was showing on.
The DB rows were abnormally empty as well.

Hm. On a test copy,

DELETE from menu_links where menu_name = '0';

fixed it for me!. (be sure to quote the '0' or you will destroy the world)

HOW it got into this state, I can't guess. Could be the menu was provided by a module that was since disabled or deleted. Or taxonomy_menu - which builds menus on the fly all the time had an aneurism at some point.

Anonymous’s picture

Anonymous (not verified) commented

I also agree that Drupal core should include a lot more assertions, input checking and waterproofing against unexpected input. But I can't convince anyone of that either.

But only for development. If a method were devised that could filter out the assertions (i.e. remove the assertions from the code) for a production ready site you might get some attention.

rooby’s picture

rooby commented

I have seen this on a number of Drupal 7 sites where a node page has been added to the management menu, under the Administration menu item, with the drupal core toolbar module enabled.
So that you have added a new item to the toolbar.

When toolbar_get_menu_tree() runs and there is a manually created node/23 type path in there you get this error.

This seems to consistently cause the problem.

I have yet to dig deeper into why.

rooby’s picture

rooby commented

It is because of this (from _menu_link_translate()):

<?php
    // menu_tree_check_access() may set this ahead of time for links to nodes.
    if (!isset($item['access'])) {
      if (!empty($item['load_functions']) && !_menu_load_objects($item, $map)) {
        // An error occurred loading an object.
        $item['access'] = FALSE;
        return FALSE;
      }
      _menu_check_access($item, $map);
    }
    // For performance, don't localize a link the user can't access.
    if ($item['access']) {
      _menu_item_localize($item, $map, TRUE);
    }
?>

Because menu_tree_check_access() has previously been called, _menu_load_objects() is not called, however _menu_item_localize() can still be called after that and it relies on _menu_load_objects() having already been run.

_menu_load_objects() should always be called before _menu_item_localize().

I will search the issue queue for existing issues for this when I get home or else open a new issue.

rooby’s picture

rooby commented

I will be following that up in #1697570: _menu_load_objects() is not always called when building menu trees