view_menu_alter() adding to existing "access arguments" causes user_access() error

Thank you I had the same problem, and the patch worked for me.

Had the same problem, the patch seems to have resolved it. Thanks.

Marking issue as needs review as other people have this problem.

Detailed backtrace and reproducible steps available at:
https://drupal.org/node/1780004#comment-7849775

Applied patch and cleared the cache. Seems to work.
I could access my broken view again.

drush sqlq 'SELECT * FROM menu_router WHERE access_callback = "user_access" AND access_arguments LIKE "a:2:%" \G'

should return nothing.

+1 patch seems to have cleared this issue for us we have had it for a very long time.

The patch had no affect on the issue for me. I'm still seeing very large numbers of these notices whenever I hit any user page. (e.g. user, user/edit, etc). I have no particular reason to think it's a views bug in my case.

My problem was that I overwrote $user. I assigned the result of entity_load('user', array($uid)) to $user instead of to $account.

Confusing typo

I tracked this down to two views trying to overtake the same path. Specifically commerce_backoffice has a admin/content view.

I was receiving this error using Views 7.x-3.7, with VBO and Admin Views.

Cache clear fixed it.

Given that this affects potentially every site we should try to write some test that this never happens again.

I am now getting this issue using the 3.8 version of views after installing admin views

Needs tests!

This patch fixed the issue for me.

Here is a downgraded patch which applies on 3.8.

@GuGuss
Are you sure we still need this patch in views itself? Afaik a patch in this area got committed already
with documentation and even a test.

@dawehner
Not sure what patch you're referring to? I'm using 3.8 and this patch fixed the notices so I guess I'll use it as it is ;-)

Though I have been using Views 3.8 and Administrations views for a while without problems, these errors started occuring after enabling Commerce Backoffice 1.9. I enabled Commerce Backoffice submodules Orders & Content at the same time, so do not know which one caused the error.

The patch from the OP views_access_arguments-0.patch works it for me so far. Thanks all!

EDIT: Both Commerce Backoffice Content and Administration views had a content view available at admin/content. After undoing the patch and disabling either of the views the problem was solved in my case.

It's not commited yet to 7.x-3.x
See: http://cgit.drupalcode.org/views/tree/views.module

Yes this patch #17 still applies, and fixes the user_access notices.

But strange my views -dev release as of today is reporting 7.x-3.7+35-dev and it should likely be 7.x-3.8...?
Maybe the 3.8 didn't get a release or something?

https://www.drupal.org/node/608852
Shows the same, so not my drush pulling it down wrong.

This basically totally drops the existing access argument, do we really want to do this?

+++ b/views.module
@@ -433,7 +433,7 @@ function views_menu_alter(&$callbacks) {
-          $callbacks[$path]['access arguments'][] = $item['access arguments'][0];
+          $callbacks[$path]['access arguments'] = $item['access arguments'];

@dawehner Likely not. Could we just check if [0] index exists before we try to append it?

if (isset($item['access arguments'][0])) {
  $callbacks[$path]['access arguments'][] = $item['access arguments'][0];
}

Related issue - #1809412: views_menu_alter adds existing access argument again. The patch in #4 of #1809412: views_menu_alter adds existing access argument again solved this issue for me.

I think that the same mechanism that defines which view/display gets shown on that path should be used to determine which access argument is used.

If two views specify the same path with different permissions, what should the access_arguments be?

Edit: It looks to me that the code in views_menu_alter() was never intended to deal with multiple views using the same path. Only with multiple displays within the same view using the same path.

I'm a bit uncomfortable without having at least proper test coverage here.

To me it looks like the current code in views_plugin_display_page::execute_hook_menu() ensures that $access_arguments[0] always is an array.
If you change e.g. the access plugin views_plugin_access_perm::get_access_callback() to not return an array of permissions the whole thing still fails - replacing the perm string by an empty array.
Now, instead fiddling with the quite complex views_plugin_display_page::execute_hook_menu() why don't we adjust views_check_perm() instead?
Change the way it operates to meet how views_check_roles() works?
Attached patch changes views_check_perm() to handle the given permission as an array - processing it with OR. Which means if there's a single permission that allows access TRUE is returned.
If the given $perms parameter isn't an array it is wrapped - this should ensure backward compatibility.
With this changes we might even able to extend the access plugin and allow to select multiple permissions.

@das-peter: the issue occurs when the menu item is inserted into the menu_router table (by views_menu_alter()) where the access_callback is set to "user_access" while the access_arguments is set to an array with multiple permissions.
views_check_perm() doesn't seem to enter the equation.

@JvE views_plugin_display_page::execute_hook_menu() which is used by views_menu_alter() can, by default, use one of the following plugins:

• views_plugin_access.inc
• views_plugin_access_none.inc
• views_plugin_access_perm.inc
• views_plugin_access_role.inc

None of those plugins returns user_access as access callback. I just can assume that if you end up with the access callback user_access that something goes wrong somewhere else.
One common issue I know about is with admin_views, it provides the display views_plugin_display_system (which uses user_access) - now if you've multiple active displays for the same system path you likely end up with a messed up access arguments array - since the arguments are merged and not replaced when altering the menu items (this is due the way how drupal_alter() is implemented).

I usually use something like this to disable such system displays provided by modules, once I switch to a customized view:

/**
 * Implements hook_views_default_views_alter().
 */
function hook_views_default_views_alter(&$views) {
  // Disable some default views of modules in favour of our own ones.
  $view_is_enabled = array(
    'admin_views_node' => FALSE,
  );

  foreach ($view_is_enabled as $view_name => $status) {
    if (isset($views[$view_name])) {
      $views[$view_name]->disabled = !$status;
      foreach ($views[$view_name]->display as $display_id => $display) {
        $display->display_options['enabled'] = $status;
      }
    }
  }
}

Thank you for the explanation.
But isn't this whole issue about a messed up access arguments array due to having multiple active displays for the same system path?

Oh if so I guess I've hijacked an issue. ;)
However, this likely means that we would need to find a strategy to handle paths that are declared twice.
I could imagine to cache what's handled already, then drop duplicated declarations and use watchdog to let the admin know about the issue.

However, this likely means that we would need to find a strategy to handle paths that are declared twice.

Well, there is actually the scope of admin_views, which provides a custom plugin to override path, which, ideally, should handle it better.

I agree that this patch of peter is a good workaround for many of the potential issues in this issue. In general its better safe to be than sorry. It could be easily the case that passing an array to user_access() accidentally returns TRUE.

I would be fine with committing as it is. It has some documentation why it handles both cases, which is really nice.
@jve Would you disagree with that entirely? I think we can leave the issue open to find a proper solution

Oh well I decided to just commit it. Let's continue with the other part of the issue :)

Some housekeeping:

• There are two patches in this issue, #17 and #29. #29 doesn't seem to address the original issue, but was committed anyway. #17 has not been committed- I'm marking that as displayed so that it's clear that it's under review.
• I was experiencing this problem with admin_views and #17 fixed it for me. It looks simple enough, I'd vote to RTBC and close this rather long-running issue.
• I think a lot of people are coming here after installing admin_views. Here is the corresponding issue that was originally in the admin_views queue and later moved to views and closed as a duplicate: #1780004: Trying to get property of non-object in user_access() with 2 admin views on the same path. I'm moving it back to admin_views for clarity.

The patch in #17 fixes the errors and missing menu items for me.

I don't know enough about the issue to judge whether or not it is the best solution.

patch #17 fixed it for me on Views Version: 7.x-3.14

The patch in #17 fixed the errors and missing menu items for me using Views 7.x-3.14.

Anyone looking for the same patch with views 7.x-3.20.

Please find the patch.

Regards,
Amjad

It seems the patch stopped working after yesterdays' new Views release (7.x-3.21) and it's happening again.

Running the tests on #43..

The patch on #43 is really the same as the patch on #7, so the patch on #43 should stop working too after the 7.x-3.21 Views release.

Patch re-rolled.

Thanks, but #43 didn't need a reroll.

No patched worked. I had to downgrade views to 7.21, now my site is back to normal. But I want my views updated, however I am currently having this massive headache of the issue kept on coming back. This could be triggered by other modules perhaps disabled/enable but I just could not identify which.

FYI If you're still seeing this problem and you use admin_views, please test the current dev version of admin_views too.

Failing that, I think debugging which view causes the problem would be useful, there might be other contrib modules which trigger the bug.

Committed. Thank you all.

Oh wait, sorry, I committed the other issue. X-)

Patch #47 get rids of all the PHP notices.

Committed. Thank you.