In pathauto.inc:pathauto_create_alias(), if the token values in $placeholders['values'] are empty because of, for example, a permissions problem, then an invalid alias is created and applied to the node. The following patch detects this case and prevents the use of the invalid alias:
*** pathauto-6.x-1.1/pathauto.inc 2008-06-18 14:02:40.000000000 -0600
--- pathauto/pathauto.inc 2009-02-12 13:45:16.000000000 -0700
***************
*** 285,295 ****
}
// Replace the placeholders with the values provided by the module,
// and optionally lower-case the result
$alias = str_replace($placeholders['tokens'], $placeholders['values'], $pattern);
!
if (variable_get('pathauto_case', 1)) {
$alias = drupal_strtolower($alias);
}
// Two or more slashes should be collapsed into one
--- 285,298 ----
}
// Replace the placeholders with the values provided by the module,
// and optionally lower-case the result
$alias = str_replace($placeholders['tokens'], $placeholders['values'], $pattern);
! if ( empty( $alias ) ) {
! // Couldn't generate an alias so do nothing
! return '';
! }
if (variable_get('pathauto_case', 1)) {
$alias = drupal_strtolower($alias);
}
// Two or more slashes should be collapsed into one
-- Walt
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | pathauto-bad-alias-6.x-1.x-dev-2009-Mar-31.patch | 611 bytes | dharmatech |
Comments
Comment #1
gregglesSuperficial review:
Can you post this as a patch file rather than inline code http://drupal.org/patch/create ?
It's also not to the http://drupal.org/coding-standards
More in depth:
Your solution to this problem seems very similar to #351639: Pathauto should not attempt to create blank aliases - prevent "The path is already in use" issues. Perhaps we should solve that issue on its own adn then revisit the idea of what to do if there is a permission failure?
Comment #2
dharmatech commentedThe attached patch against pathauto-6.x-1.x-dev of 2009-Mar-31 should conform to standards. It was tested as follows:
1. Installed pathauto-6.x-1.x-dev (2009-Mar-31).
2. Verified that automatic aliases were generated correctly with 'workflow access' module disabled.
3. Enabled 'workflow access'.
4. Tried to create another page with an automatic alias and got this error message:
user warning: Duplicate entry '/feed-en' for key 2 query: INSERT INTO url_alias (src, dst, language) VALUES ('node/21/feed', '/feed', 'en') in /home/devl/public_html/modules/path/path.module on line 112.
5. Installed the attached patch.
6. Repeated steps 2-4. No alias was generated, including the dst='' alias. No error message appeared.
From studying the code, it appears that this would also solve the problem of Acidfree generating a blank alias, but this has not been tested.
-- Walt
Comment #3
gregglesThanks for the much more thorough explanation that makes this easy to understand.
I think that the expected result here should be that Pathauto somehow creates the alias rather than just failing to create it.
So, I'd much rather that we do something like safely impersonate another user and pretend to be UID 1 so that we can get aliases for these.
Comment #4
dharmatech commentedOops, in #2 above I forgot to mention some conditions needed to replicate the problem:
Path pattern applicable to the node being created is [menupath-raw]
The user creating the node that has this problem is NOT user 1 and DOES NOT HAVE 'administer nodes' permission.
In this situation, inserting a node calls pathauto_get_placeholders() which calls token_get_values() which calls module_invoke_all('token_values',...) for the node being inserted. This results in a call to node_token_values() which retrieves the menu_link ID for the menu item being created and eventually calls _menu_check_access() to see whether the user inserting the node has access rights to this menu item. This calls node_access(), which finds that the user does not have 'administer nodes' access, so it calls all hook_node_grants() implementations. workflow_access_node_grants() returns a list of grants. The node_access table is duly searched for a grant to view this node, but since the node is still in the process of creation it has not yet been added to node_access, so of course no access rights are returned for it. This eventually causes a null string to be returned for the [menupath] and [menupath-raw] tokens, which in turn causes a null alias to be generated.
The problem then is the exact sequence of checks. If the node had been inserted in node_access before the call to _menu_check_access() then the node_access table would allow access and the expected values would be returned for [menupath] and [menupath-raw]. Or, if no implementations of hook_node_grants were enabled, the node_access table would not be checked and again the expected token values would be returned.
The patch attached to #2 above does nothing to correct this, it merely prevents insertion of a garbage URL alias.
Cheers -- Walt
Comment #5
gregglesRight. It would be better if it fixed the problem rather than the symptom.
Comment #6
dave reidAs already pointed out by Greg, we already have issue #351639: Pathauto should not attempt to create blank aliases - prevent "The path is already in use" issues. Issues about the menupath token should be filed against Token module since Pathauto does not provide those tokens. Marking as fixed/duplicate.