[development] hook_menu split is inevitable
karoly at negyesi.net
Tue Jan 23 08:49:50 UTC 2007
> 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)?
Yes. We need to do two access control, let's say an "access control light" for hook_menu and that's permissions -- as I have outlined this is more than what Drupal 5 does for custom menu items. And an "access control full" and that's callbacks for hook_router.
> > 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*
The sorting for hook_router is based on number_parts (which in the above example for foo/bar/this is three) but hook_menu needs depth (which in the above example for foo/bar/this is one) -- and you can only sort on one or the other.
> 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
It is a good foundation to build on it. The router functionality as implemented can and will stay unchanged aside from the small fact that its hook will be renamed. Please note that this is exactly what I planned for this phase. You asked for a navigation block so that Drupal development remains easy and I spent more than a full day doing one. It works quite nicely until you start giving acccess to a role to some item without giving access to its parents or defining a tree of paths with visible and invisible items intermingled -- core does not have such a one. There are no unknowns -- I know of every problem and have a plan to fix them.
The reason I can't do the right thing for the link functionality is that there are at least two other patches (Steven's url arguments array patch and the normalize permissions) that I depend on and one of them (the norm.perm. one) is nowhere near finished.
Can we hold this patch and wait for the others to finalize? Of course that's a possibility which I really would not like because currently the menu patch works and as features will be committed it will be quite a task and a waste of time to reroll and reroll a 200K+ patch to keep up with HEAD.
More information about the development