_menu_check_access() does not warn when the access callback does not exist

@Damien -- Thanks. I never know whether these variable keys are supposed to be defined, or whether the original programmer had E_NOTICE turned off.

Working on reproducing the issue, starting over with a blank database and reinstalling module-by-module, with the following code inserted:

    if (!isset($item['access'])) { error_log("access array key missing from item " . print_r($item,1) . ", backtrace:" . print_r(debug_backtrace(0),1)); }

It's entirely possible that the original error happened during my abortive attempt to do a D6--D7 upgrade. If I can't reproduce on a clean install I'll close this issue, and thanks again for your input.

Okay, the error is definitely triggered by the installation of admin_menu. Moving to that project.

Unfortunately, starting from a clean install and then adding admin_menu and admin_menu_toolbar, the problem doesn't trigger. So it's a combination of two or more modules that don't work together (sigh).

Still narrowing it down...

Okay, here's the minimal steps to reproduce:

1. Install Drupal-7 HEAD from CVS with the "Minimal" profile.
2. Patch includes/menu.inc as follows:
   

   Index: includes/menu.inc
   ===================================================================
   RCS file: /cvs/drupal/drupal/includes/menu.inc,v
   retrieving revision 1.427
   diff -u -r1.427 menu.inc
   --- includes/menu.inc   2 Dec 2010 20:18:23 -0000       1.427
   +++ includes/menu.inc   11 Dec 2010 23:27:01 -0000
   @@ -891,6 +891,7 @@
          _menu_check_access($item, $map);
        }
        // For performance, don't localize a link the user can't access.
   +    if (!isset($item['access'])) { error_log("access array key missing from item " . print_r($item,1) . ", backtrace:" . print_r(debug_backtrace(0),1)); }
        if ($item['access']) {
          _menu_item_localize($item, $map, TRUE);
        }
   @@ -1424,6 +1425,7 @@
      foreach ($tree as $key => $v) {
        $item = &$tree[$key]['link'];
        _menu_link_translate($item);
   +    if (!isset($item['access'])) { error_log("access array key missing from item " . print_r($item,1) . ", backtrace:" . print_r(debug_backtrace(0),1)); }
        if ($item['access'] || ($item['in_active_trail'] && strpos($item['href'], '%') !== FALSE)) {
          if ($tree[$key]['below']) {
            _menu_tree_check_access($tree[$key]['below']);
   

3. Enable the "Field UI"module.
4. Install the "Administration menu" module from CVS and enable it.

Attached:

• database dump immediately before enabling "Administration menu" module.
• error_log output from above debugging code (truncated to max upload filesize).
• database dump immedately after enabling "Administration menu" module.

Weird. Now I can't duplicate...

Wonder if the problem was due to Google Chrome's "autocomplete prediction service" which I have subsequently turned off.

EDIT: Nope. Happened again as soon as I enabled some more modules... (sigh) Still debugging...

Okay, I'm moving this back to Drupal core because the error seems to be in the field_ui module.

Here's the sequence of events:

1. in function _menu_translate() of includes/menu.inc, the documentation guarantees that $item['access'] will be either TRUE or FALSE when the function exits. To fulfill this guarantee, it calls

     _menu_check_access($router_item, $map);
   

   near the end of the function.

2. in function _menu_check_access() of includes/menu.inc, the following code is executed:
   

         $item['access'] = call_user_func_array($callback, $arguments);
   

   with $callback = '_field_ui_view_mode_menu_access' and $arguments=array('user', 'user', 'full', 'user_access', 'administer users')

3. In _field_ui_view_mode_menu_access() of modules/field_ui/field_ui.module, the following code:
   

   function _field_ui_view_mode_menu_access($entity_type, $bundle, $view_mode, $access_callback) { 
     // First, determine visibility according to the 'use custom display' 
     // setting for the view mode. 
     $bundle = field_extract_bundle($entity_type, $bundle); 
     $view_mode_settings = field_view_mode_settings($entity_type, $bundle); 
     $visibility = ($view_mode == 'default') || !empty($view_mode_settings[$view_mode]['custom_settings']); 
     
     // Then, determine access according to the $access parameter. This duplicates 
     // part of _menu_check_access(). 
     if ($visibility) {
   # ... (intervening code omitted for brevity) ...
     }
   }
   

   results in $visibility being FALSE, so the entire body of the function is skipped and no value is returned at all.

I believe that this is either a bug in _menu_translate (for relying on access callbacks to fulfill its contract) or in field_ui (for failing to set access in _field_ui_view_mode_menu_access).

One-line patch.

Better title.

@gamerxgirl -- probably there is another helper function being called that also fails to return a value. IMHO, _menu_translate should issue a warning when its helper function fails to return a value.

patch attached.

EDIT: Ignore http://drupal.org/files/issues/menu.inc-Undefined-index--access--in--ite...
It was checking in the wrong place.

Fresh title

Need to read the api docs for watchdog() more carefully...

testbot

Trying again...

better title

@Damien -- Try running the following code:

function no_value_returned()
{ /* Do nothing and return no value */
}

$foo['bar'] = no_value_returned();

if (isset($foo['bar']))
  { print "The function returned a value.\n";
  }
else
  { print "The function did not return a value.\n";
  }

PHP 5.3 prints "The function did not return a value." Perhaps your PHP behaves differently.

The router item with its NULL value gets serialized and stored in the database, then unserialized and used later.

Try this:

ini_set('display_errors',1);
ini_set('error_reporting',E_ALL);

function no_value_returned()
{ /* Do nothing and return no value */
}

$foo['bar'] = no_value_returned();

$serialized = serialize($foo);

$foo = unserialize($serialized);

$bar = $foo['access'];

Bah. You're right. But doggone it, SOMEWHERE it was producing errors the other day. Will keep trying. Meanwhile I still assert that the function fails to fulfill its contract, because it promises either TRUE or FALSE and delivers NULL, which is neither.

I'm trying to avoid another six hours sifting through megabytes of debug output trying to nail down why it is that when $item['access'] is set to NULL rather than TRUE or FALSE, a notice later occurs.

Good catch.

Re-rolled for fuzz.

Removed what I now understand to be a superfluous watchdog call and re-rolled.

Patch.

Better title.

Browsing the issue queue, I like the approach in #1059884: Drop function_exists for 'callback' functions better. Marking this issue as duplicate.

Re-opening for 7.x since the 8.x fix will not be backported.

#37: includes_menu.inc_.patch queued for re-testing.

Re-rolled.

@#46 I don't think your error is related to this issue. If you disagree, please walk the rest of us through the code path you think is occurring.

#45: menu_check_access-994992-45.patch queued for re-testing.

Updated issue summary. Either this patch or a backport of #1059884: Drop function_exists for 'callback' functions should be reviewed and committed.

Re-rolled as requested.

Putting a little isset in there fixes it...

Patch?

If this is a feature request, then it should be marked "wontfix" because the "feature" of checking for function existence was removed in 8.x.

Better issue summary.