I'm attempting to determine the group context in a menu access callback using og_context(). If that function is called in any menu access callback, Drupal crashes from what seems to be a memory issue. Is there a better way to determine the current group (without using URL arguments)?

Comments

mstef’s picture

mstef commented

Seems to be because of the call to menu_get_item(), more importantly, the call to _menu_translate(), then _menu_check_access().

Suggests an infinite loop?

mstef’s picture

mstef commented

Same issue when calling menu_get_object();

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

I see.. What's the use case, maybe we can find a work around

mstef’s picture

mstef commented

Well, my use-case is a little rare, but when I think about, it probably isn't..

I can see there being a lot of pages/forms/etc that will need to check what the current group is, and dictate access based on that. Not every page will have an entity type and entity id passed into it, where we can load the group in question, then check access from that.

I'm sure developers are used to og_get_group_context() from OG6, which was (almost) always available (though, it wasn't very reliable).

My use case is for OG features (working on the OG7 port right now), but this is critical. The module actually works by overridding the access callback of every path in the system. If the current page is within a group context, access will be denied if the page is part of an OG feature that is disabled for the current group, otherwise the normal access callback is invoked. So, I need the group context available, always (if there is one). --- rare use case, but still general enough where I think this needs to be fixed.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Category: bug » support

You can use og_context_determine_context() that can get the $item as a paramter -- it will be up to you to populate that item, when you hook_menu_alter()

mstef’s picture

mstef commented

No, that's not the case..

First, in order to get the $item, menu_get_item() needs to be called (even if it's from the access callback). That will result in this issue.

Second, what I did was duplicated menu_get_item(), and removed all access calls to prevent the endless loop. The problem is now, that og_context_determine_context() will then call og_context_handler_node(), which calls both menu_get_object() and menu_get_item(), which will result in this issue.

I saw you changed this to a support request. I still think it's a bug (for the record).

What do you think?

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

> The module actually works by overridding the access callback of every path in the system.
...
> First, in order to get the $item, menu_get_item() needs to be called

But if I understand correctly, you use hook_menu_alter() to change all the path, so at that point you can append to the access arguments the original item -- like this you won't have to call menu_get_item() yourself (that's why I changed to support -- if you think otherwise you can change back of course)

mstef’s picture

mstef commented
Category: support » bug

Well first, forget about my weird use-case, because this happens if you put it in any single menu access callback. I don't want that to confuse the situation.

But if I understand correctly, you use hook_menu_alter() to change all the path, so at that point you can append to the access arguments the original item

The fully-loaded menu item is not passed into the menu access callback. menu_get_item() has to be called because many important things come with it. I see that og_context_determine_context() only uses $item['path'], so I could pass in array('path' => $path), to avoid the call, but like I said, og_context_determine_context() will automatically call og_context_handler_node(), which will call both menu_get_item() and menu_get_object() (this cannot be avoided).

I'm putting this back as a bug because although it does work, if used in certain (relatively normal, expected) ways will break badly.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

> if used in certain (relatively normal, expected) ways will break badly.

You'll have to suggest an idea, because I don't know other ways of getting current path without using menu_get_item()..

mstef’s picture

mstef commented

This is a pretty nasty hack, but it's the only thing I've been able to do so far as I'm messing with this issue..

Make your own copies of menu_get_item() and _menu_translate()..

In menu_get_item(), you'd have to change the call to _menu_translate() to the copied function you made. You'll also have to remove the IF clause on line 477 (if ($router_access['access'])).

In _menu_translate(), you'll have to remove lines 788-793.

Basically that just removes the access checks, which we don't need because we're just looking for context. The menu system will deny access regardless.

I know this is really ugly, and I wouldn't want this committed in my module, but it just serves to show what the problem is, and what *does* fix it (not the best way to fix it). I'm actually really surprised that no one else came across this.

jgraham’s picture

jgraham commented

I just came across a similar issue, albeit in 7.x-2.x-dev. The below call stack is the infinite loop section for us.

    0.0572    4579004   6. menu_get_item() /srv/www/vbdev/html/includes/menu.inc:1754
    0.0572    4579288   7. menu_rebuild() /srv/www/vbdev/html/includes/menu.inc:459
    0.0584    4580036   8. menu_router_build() /srv/www/vbdev/html/includes/menu.inc:2712
    0.1657    7968644   9. drupal_alter() /srv/www/vbdev/html/includes/menu.inc:2752
    0.1677    7974104  10. views_menu_alter() /srv/www/vbdev/html/includes/module.inc:1056
    1.2190   16419744  11. view->execute_hook_menu() /srv/www/vbdev/html/profiles/ghel_install/modules/contrib/views/views.module:401
    1.2191   16478004  12. views_plugin_display_page->execute_hook_menu() /srv/www/vbdev/html/profiles/ghel_install/modules/contrib/views/includes/view.inc:1448
    1.2492   16792068  13. og_context_plugin_access_og_perm->get_access_callback() /srv/www/vbdev/html/profiles/ghel_install/modules/contrib/views/plugins/views_plugin_display_page.inc:82
    1.2493   16792068  14. og_context() /srv/www/vbdev/html/profiles/ghel_install/modules/contrib/og/og_context/includes/views/handlers/og_context_plugin_access_og_perm.inc:29
    1.2493   16792344  15. og_context_determine_context() /srv/www/vbdev/html/profiles/ghel_install/modules/contrib/og/og_context/og_context.module:200

We had also encountered this issue with og_workflow but I was able to work up a patch that routed around this issue. #1827420: infinite recursion caused by og_workflow_user_roles_for_node() when using og_workflow_access

Now we have exposed the same issue with views; I'm hoping that we can just modify the view to avoid this.

If I find any more pertinent details I will be sure to add them here.

EDIT: FWIW we removed the following from the view and it started working again.

- $handler->display->display_options['access']['type'] = 'og_context';
- $handler->display->display_options['access']['perm'] = 'manage members';

shenzhuxi’s picture

shenzhuxi commented

#11 seems because views called _og_context_views_page_access which called menu_get_item which called _og_context_views_page_access again when a view is saved.
Patch http://drupal.org/node/1606990#comment-6291286 seems fix the infinite loop during saving a view.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Active » Closed (duplicate)
mstef’s picture

mstef commented

Patch http://drupal.org/node/1606990#comment-6291286 seems fix the infinite loop during saving a view.

Yea, but does it solve the problem that originally led to this thread? If not, this shouldn't be marked as a duplicate.