Problem/Motivation

There is a problem when inserting/updating a node when this module runs alongside Menu Node API

In Menu Node API's _menu_node_invoke function, there's

...
module_invoke_all('menu_node_' . $hook, $link, $node);

Where $hook can contain 'update/insert/delete' resulting in a module_invoke_all('menu_node_update/insert/delete').

The problem is that Drupal thinks that the og_menu module implements those hooks whereas it only implements hook_node_update/insert/delete (that's because there is a module called og and a function called og_menu_node_update even though the code does not belong to the og module)

Since a hook_node_update/insert/delete expect a $node as its only parameter, the $link provided by menu_node's module_invoke_all('menu_node_' . $hook, $link, $node) ends up in a EntityMalformedException.

I already created a ticket in Menu Node API's issue queue for this problem (see EntityMalformedException is thrown due to an unwanted call to hook_node_update in og_menu) but I think the issue lies here and not in their module.

Proposed resolution

In the aforementioned ticket, I didn't know what was the best option to resolve this issue, but now I think you should test the parameters you receive in your hook_node_update/instert/delete implementations to avoid any issues.

Remaining tasks

n/a

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Babou created an issue. See original summary.

Babou’s picture

Here is a first attempt to patch the issue.

The patch is bit "weird" for the hook_node_delete implementation as I just added a line on top to test the parameters and a closing brace at the end.

rv0’s picture

dasginganinja’s picture

Version: 7.x-3.0 » 7.x-3.x-dev

Shouldn't this just be fixed in core? The module_invoke_all($hook) implementation in includes/module.inc loops though the output of module_implements($hook) so the issue really seems that is should be fixed there.

@rv0 It seems like og_menu could at least put checks in to check the number of arguments / validate that it's not a mlid that is being passed to it.

Thoughts? I just got burned in a bad way by this. This still exists in og_menu-7.x-3.1.

rv0’s picture

Not sure how this could get fixed in core. The problem is poor naming on menu_api's side

I agree though about some light weight checks, feel free to suggest a patch.

dasginganinja’s picture

FileSize
1.34 KB

@rv0 this could be handled by fixing core's implementation of module_implements($hook) to check which module the hook is really defined in.

For the time being here is a patch that checks if menu_node is enabled and that the number of arguments match the expected signature.

dasginganinja’s picture

Status: Closed (duplicate) » Active
dasginganinja’s picture

Status: Active » Needs review