As of now, a person could create a new node with a URL alias that overrides a menu_router entry. For example, one could create an article with the alias "node/add/page". Then when attempting to create a page, they would be redirected to the offending article.

It occurs to me that if a path is in the menu_router table, it should not be eligible as a URL alias.

CommentFileSizeAuthor
#5 path_validate_menu_router-1561372-5.patch741 bytesKevin Hankens
#1 path_validate_menu_router-1561372-1.patch972 bytesKevin Hankens
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Kevin Hankens’s picture

Kevin Hankens CreditAttribution: Kevin Hankens commented
FileSize
path_validate_menu_router-1561372-1.patch972 bytes

Adding patch.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Active » Needs review

Patch makes sense to me, let's see what testbot thinks.

Status: Needs review » Needs work

The last submitted patch, path_validate_menu_router-1561372-1.patch, failed testing.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

The failures are in PathAliasTestCase

Attempt to moved alias was rejected.
"The alias is already in use." found

I didn't dig in, but it seems like test is relying on this bug...?

Kevin Hankens’s picture

Kevin Hankens CreditAttribution: Kevin Hankens commented
FileSize
path_validate_menu_router-1561372-5.patch741 bytes

Oh, that was my bad. I changed the dsm to have a more friendly message. The assertion was looking for the old error message.

This patch uses the old messages instead.

Kevin Hankens’s picture

Kevin Hankens CreditAttribution: Kevin Hankens commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, path_validate_menu_router-1561372-5.patch, failed testing.

Kevin Hankens’s picture

Kevin Hankens CreditAttribution: Kevin Hankens commented
Status: Needs work » Needs review

#5: path_validate_menu_router-1561372-5.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, path_validate_menu_router-1561372-5.patch, failed testing.

Kevin Hankens’s picture

Kevin Hankens CreditAttribution: Kevin Hankens commented

I don't understand these failures, the only thing that changed from the first patch was the argument to form_error. Although, this did fix the path tests :) Any thoughts?

Kevin Hankens’s picture

Kevin Hankens CreditAttribution: Kevin Hankens commented

It appears that these five failures also happen locally on a fresh install /without/ the patch above.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

I wonder if people use this feature to "take over" paths generated by modules.

chx’s picture

chx CreditAttribution: chx commented

Thanks for trying to fix this but the approach taken is not acceptable. Going around an API to work with a database table directly is, in general, wrong. Here, I would run menu_get_item() first and then take a good look at the resulting $item['path']. This would deal with the problem of having a path foo and an alias foo/bar. Something rather similar to http://api.drupal.org/api/drupal/includes%21path.inc/function/drupal_val... except you do not need to deal with dynamic path but need to be able to look at the $item itself.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

Hmmm, yeah, moshe is right. We shouldn't prevent that in path.module.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
CreditAttribution: cweagans commented

There's already a good way to take over paths generated by modules. I don't think it makes any sense to allow somebody to create a node over the top of an existing menu_router path - at least, not without warning them.

Maybe this is a silly example, but what if somebody created a webform over the top of /user/login and started collecting user credentials from the users of the site? That would be less than ideal.

nmc’s picture

nmc CreditAttribution: nmc commented

I'd like to see this "fixed" in core as well.

Along the same lines as @chx, why not do as Pathauto does in its _pathauto_path_is_callback?

Here's the revised version:

function _pathauto_path_is_callback($path) {
  $menu = menu_get_item($path);
  if (isset($menu['path']) && $menu['path'] == $path) {
    return TRUE;
  }
  elseif (is_file(DRUPAL_ROOT . '/' . $path) || is_dir(DRUPAL_ROOT . '/' . $path)) {
    // Do not allow existing files or directories to get assigned an automatic
    // alias. Note that we do not need to use is_link() to check for symbolic
    // links since this returns TRUE for either is_file() or is_dir() already.
    return TRUE;
  }
  return FALSE;
}

The second part of the code above seems ideal to me as well. That way we can avoid these situations: http://groups.drupal.org/node/20604

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Issue summary: View changes
Issue tags: +Needs issue summary update, +Needs tests

Just verified on 11.x that this can still happen.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs work » Closed (duplicate)
Related issues: +#121362: Do not allow existing or reserved paths as aliases

Actually seems like a duplicate