Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The code to determine the list of un-aliased paths in drupal_lookup_path() is:
$cache['no_aliases'][$path_language] = array_flip(array_diff_key($cache['system_paths'], array_keys($cache['map'][$path_language])));
Given the following path arrays:
$cache[system_paths] ==>
Array
(
[0] => node/1
[1] => node/2
[2] => node/3
[3] => node/4
[4] => node/5
[5] => user/2/edit
[6] => /user/logout
[7] => taxonomy/term/1
[8] => taxonomy/term/2
[9] => taxonomy/term/3
[10] => taxonomy/term/4
[11] => taxonomy/term/5
)
$cache[map][$path_language] ==>
Array
(
[node/1] => node-1
[taxonomy/term/1] => taxonomy-term-1
[taxonomy/term/2] => taxonomy-term-2
[taxonomy/term/3] => taxonomy-term-3
)
the above code has the un-aliased list as:
$cache[no_aliases][$path_language] ==>
Array
(
[node/5] => 4
[user/2/edit] => 5
[/user/logout] => 6
[taxonomy/term/1] => 7
[taxonomy/term/2] => 8
[taxonomy/term/3] => 9
[taxonomy/term/4] => 10
[taxonomy/term/5] => 11
)
when in fact the list should be:
$cache[no_aliases][$path_language] ==>
Array
(
[node/2] => 1
[node/3] => 2
[node/4] => 3
[node/5] => 4
[user/2/edit] => 5
[/user/logout] => 6
[taxonomy/term/4] => 10
[taxonomy/term/5] => 11
)
Proposed resolution
Fix the expression by replacing array_diff_key()
with array_diff()
.
Remaining tasks
Review and test.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#5 | 2058885-5.patch | 898 bytes | raman.b |
#3 | 2058885-3.patch | 1.14 KB | Oscaner |
#1 | 2058885-cached-un-aliased-paths.patch | 1.07 KB | solotandem |
Comments
Comment #1
solotandem CreditAttribution: solotandem commentedAttached patch implements proposed resolution.
Comment #2
dzutaro CreditAttribution: dzutaro commentedThis performance bug still exists in Drupal 7.50.
Comment #3
Oscaner CreditAttribution: Oscaner at CI&T commentedThis performance bug still exists in Drupal 8.7.x. But not in drupal_lookup_path() function, it's in the AliasManager class.
The uploaded images are my debug informations.
Comment #4
LendudeMoving to the right version
Comment #5
raman.b CreditAttribution: raman.b at OpenSense Labs commentedAdding a patch for the current dev branch and tagging for tests
Comment #6
SpokjeHmmm, the fact that path #5 passes without TestBot complaining seems to point to the fact that there was no test-coverage of
getAliasByPath
for this specific case.Anyway, as @raman.b correctly pointed out, there must be tests for this issue.
Let me see if I can come up with some.
Comment #7
SpokjeTried to come up with tests, but couldn't find a decent way to test this.