[drupal-devel] [bug] Path module allows one to override existing (non-aliased) paths.

[andremolnar]

 Project:      Drupal
 Version:      cvs
 Component:    path.module
 Category:     bug reports
 Priority:     critical
 Assigned to:  Anonymous
 Reported by:  adrian
 Updated by:   andremolnar
 Status:       patch

+1 for adding the menu_item_exists function even with its limitations
that adrian has pointed out.
"* When an additional module gets enabled, any already specified links
will take precedence over the new links created by it.
"
Could you include a similar check when enabling a module.  i.e. a
function called from module.module that would look at the menu
callbacks of the newly enabled module, check them against the menu
array and post an warning on the admin/module page and add an entry to
watchdog.  Admins would at least be notified about the problem and
could take appropriate action.
e.g. Warning: A path alias exists for $path.  This path is in conflict
with a registered path in the newly enabled module $module.  This
conflict will not allow module $module to function properly.  The
problem can be corrected by renaming $path to something else from
linktoadmin/alias.
and watchdog could have something like a user error with a short
description "path conflict" and a detailed message similar to the
example above.
andre


andremolnar



Previous comments:
------------------------------------------------------------------------

February 15, 2005 - 20:51 : adrian

Attachment: http://drupal.org/files/issues/path_menu_exists.diff (2.27 KB)

One of our users recently tried to link to /admin , using the path
module. And the link was allowed to be created.
This meant his admin menu was disabled, which should not be able to
happen.
Attached is a function added to menu.inc that steps through all the
menu items, and finds out if there is a callback registered for a
specific path, unfortunately there still exists a few problems :

Only explicitly registered callbacks are detected. For instance: 'node'
will be picked up, but 'node/12' will be allowed. This is not as easy to
test for, because of how pages default back to the top most callback,
and pass the rest of the fields as paramaters. The only way to work
around this is to explicitly define all callbacks that use parameters
with a 'paramaters'=> true in the item.
When an additional module gets enabled, any already specified links
will take precedence over the new links created by it.



------------------------------------------------------------------------

February 15, 2005 - 22:47 : Axel

I think just not allow to users change paths - this instrument is for
site admins/editors only. Additional checks will slow down path module.
If site admin want to allow users change paths to their own manner then
such job must doing by separate module, which will offer interface for
change paths with restrictions and additional checks.


------------------------------------------------------------------------

February 15, 2005 - 23:18 : Boris Mann

When adrian said "user", he meant a site admin who is using one of our
hosted installs.
The issue remains: anyone can alias "admin" or any other existing path,
which will override existing pages.


------------------------------------------------------------------------

February 16, 2005 - 01:50 : Bèr Kessels

"  Additional checks will slow down path module." True. But IMO thats 
non-issue in the rare case of a path added. How will adding 100
aliasees a day (not sure if any site out there makes that) affect
performance?
Performance issues on adminstrative functions should have a very low
priority. Below all others like safety, useability and all.
For the rest: I agree upon a separate advanced path module.
We have more "unsafe" issues, such as people being able to add PHP,
block themselves, block Superuser, etc. A line of documentation on this
might suffice?


------------------------------------------------------------------------

February 16, 2005 - 04:31 : Anonymous

administrators are users too. =)
And this stops the most common abuse, but there are still special cases
I mentioned.
Also, there is exactly 1 for loop extra and an extra if statement, only
upon submission of a new path. I hardly think it is a performance issue.


-- 
View: http://drupal.org/node/17386
Edit: http://drupal.org/project/comments/add/17386