[development] hook_menu split is inevitable
dries.buytaert at gmail.com
Tue Jan 23 07:47:04 UTC 2007
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
Dries Buytaert :: http://www.buytaert.net/
More information about the development