Hi,

I just passed a couple of days working on an issue in a project and wanted to share my findings and report a bug ( from my point of view ). I am quite new to drupal.org so I might have filed this issue under the wrong component (was looking for path system).

What I found out is that two consecutive calls to drupal_lookup_path don't give the same result when asking for an alias, let me explain what i mean whith a simple example :

$path = 'node/10'; //let's assume that node/10 has many different aliases
$alias1 = drupal_lookup_path('alias', $path); //here alias is the most recent one that can be found for node/10 in table url_alias
$source = drupal_lookup_path('source', 'not_the_most_recent_alias_of_node/10'); //source is node/10
$alias2 = drupal_lookup_path('alias', $path); //alias2 is the alias that was previously passed into drupal_lookup_path looking for source

I consider this as a bug because it doesn't seem right that drupal_lookup_path does the work to find the most recent alias on first call and this work can get overriden with another value. I would expect $alias2 to be equal to $alias1 if no change to the table url_alias has been done.

I am aware that this might be by design and that performance issues must be very important at that level considering how many times this function is called but is it possible to, without destroying the performance, having a consitent behavior?

My idea out of the blue to resolve this issue with minimal performance impact would be to replace : 


if ($source = $result->fetchField()) {
          $cache['map'][$path_language][$source] = $path;
}

by : 

if ($source = $result->fetchField() && empty($cache['map'][$path_language][$source])) {
          $cache['map'][$path_language][$source] = $path;
}

This would avoid the inconsistency in my opinion but I am unsure of the validity of this solution or the impact it could have.

Just to add a bit of practical info, i fell onto this problem when working with boost and globalredirect, and this issue makes globalredirect work only partially (considering the usage statistics of both, I imagine other people are experiencing the same thing).

I would really appreciate your insights on this matter.

CommentFileSizeAuthor
#7 drupal-drupal_get_path_inconsistencies-2212297-7-D7.diff2.69 KBjamesdevware

Comments

grhodes’s picture

grhodes commented
Issue summary: View changes
grhodes’s picture

grhodes commented
Issue summary: View changes
grhodes’s picture

grhodes commented
Issue summary: View changes
grhodes’s picture

grhodes commented
Title: drupal_lookup_path not giving same results for identical calls » drupal_lookup_path has an inconsistent behavior
grhodes’s picture

grhodes commented
Version: 7.25 » 7.26

Just changed the version because this issue also affects 7.26

grhodes’s picture

grhodes commented
Component: base system » path.module

Other issues talking about drupal_lookup_path use path.module component.

jamesdevware’s picture

jamesdevware commented
Version: 7.26 » 7.54
Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new drupal-drupal_get_path_inconsistencies-2212297-7-D7.diff2.69 KB

This is also an issue if you call drupal_lookup_path('source', 'non-canonical-alias') and then drupal_lookup_path('alias', 'internal-path') this will return you the non-canonical-alias that you previously passed to drupal_lookup_path.

To re-produce this issue you can simply set up a node with 2 aliases.
Then run the following using the alias with the lowest pid.

$non_canonical_alias = 'non-canonical-alias';

$source = drupal_lookup_path('source', $non_canonical_alias);
$a1 = drupal_lookup_path('alias', $source);
drupal_static_reset('drupal_lookup_path');
$a2 = drupal_lookup_path('alias', $source);

dpm($a1);
dpm($a2);

This causes race conditions and is causing strange interactions between contrib modules.

There are multiple issues in the queue that would likely be fixed by this change.

Patch is attached to ensure that when retrieving the source for an alias we self join so that we can add the canonical_alias to the cache. This means that we can safely set the cache as the results are consistent between the two actions.

Status: Needs review » Needs work

The last submitted patch, 7: drupal-drupal_get_path_inconsistencies-2212297-7-D7.diff, failed testing. View results

Version: 7.54 » 7.x-dev

Core issues are now filed against the dev versions where changes will be made. Document the specific release you are using in your issue comment. More information about choosing a version.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.