[development] hook_menu split is inevitable

[Dries Buytaert]

On 21 Jan 2007, at 23:53, Karoly Negyesi wrote:
> Try this: give administer comments permission to anonymous (no  
> other admin perms) and log out . Observe the comments link in the  
> navigation block on the home page in the Navigation block for  
> Drupal 5. The new patch will not even consider admin/content/ 
> comment for display. To do so, it would need to access check every  
> single visible item in the menu table. As the new system does  
> access checking on run time via callbacks, this will lead to  
> slowdowns. We could attack this with various caching strategies but  
> that's not the solution.
>
> The simple fact is that hook_menu need and hook_router should not  
> use the same permission system. hook_menu should only do  
> permissions, after http://drupal.org/node/73874#comment-116054 this  
> patch is done it will be just a JOIN. This way, in a sense,  
> hook_menu indeed can access check every single menu item. We will  
> give the permissions a numeric ID and the JOIn can happen over  
> menu_links, permissions_roles, user_roles -- a three table JOIN  
> over numeric keys. Likely to be reasonably fast.

Are you saying that hook_menu, rather than hook_router, should do the  
access control?  This sounds a little bit scary.  It sounds as if  
securing things at the router level would be less prone to security  
errors.  Or are you saying that both hook_router and hook_menu need  
to duplicate access control (in a different way)?

> There are other fun problems with visible links and router items  
> mingled into one: if foo is not defnied, foo/bar is not visible but  
> foo/bar/this is visible and again foo/bar/this/that is not visible  
> then how should we sort these paths? For inheritance purposes, the  
> order I listed them is a must, but visibility purposes foo/bar/this  
> actually appears on the first level. This is another sure sign that  
> menu and router structures are very much not the same.

How so?  *confused*

> I still propose to get the patch in as it stands. Before the  
> navigation block was added back it was actually hook_router and it  
> was pretty much without problems. Accept that the navigation block  
> I added back yesterday is just a development tool and not the Real  
> McCoy yet -- eventually the quirks above will be smoothed out. It's  
> sheer impossible to do all changes at once.

I'm a little bit nervous about committing a patch that has many  
unknown edge-cases.  What makes it difficult to extend this patch?   
We gave it a several reviews and the code looks good -- it should be  
a good foundation to build on -- whether it is actually committed or  
not.

--
Dries Buytaert  ::  http://www.buytaert.net/