Obscuring true paths of redirects is confusing and buggy

Yes. The i18n module does some nasty things to Drupal's URLs. I'm not sure that I can make the two modules compatible in Drupal-5. Things should be better in Drupal 6, but I'm sure that doesn't help your situation much.

Yikes. That does sound like more of a problem with the i18n module than the path_redirect module!

The problem is fixable in path_redirect. But it would be the best to change its concept a little before.

What does path_redirect with i18n at the moment:

Something like en/node/1 -> en/node/2 [301] would redirect without checking the language, because the language isn´t stored in the database. It´s only the actual language which is rendered in front of the path. This is usefull if you want a redirect independent of the selected language.

But I would prefer a possibility to select if I want it independent or not. For example I needed a redirect for the following case en/node/1 -> de/node/2 [301]. I made several changes to the code to change the standard behaviour to this case. I will post this changes later.

Greats from Germany
LeisureLarry

I´ve changed the functions path_redirect_init and path_redirect_edit_validate.

In the first one I´ve added code at the beginning and in the second one I´ve disabled the lines which are using drupal_get_normal_path.

Now path_redirect will always do a language specific redirect, i.e. en/node/1 -> de/node/2 [301].

The best would be to make this a selectable feature. For each redirect a choice between language dependent and independent would be great, but this would require a change in the database table of path_redirect.

Greats from Germany
LeisureLarry

Hi there, LeisureLarry. Any chance you could post your changes as a patch instead?

By the way, I have also found this line in path_redirect_edit_validate to be problematic:

$form_values['path'] = drupal_get_normal_path($form_values['path']);

I'm not sure why it's there. It seems like it means that the database isn't actually storing what I typed in, but rather some other path which may not be what I intended. It seems to work fine (perhaps even better) without it, but maybe I'm missing the problem. I think I had to take that out entirely to get it to work in the D6 branch.

Sorry, I´m not able to make it a patch. But I can tell you what I´ve changed so far.

function path_redirect_edit_validate:

deleted two lines
$form_values['path'] = drupal_get_normal_path($form_values['path']);
$form_values['redirect'] = drupal_get_normal_path($form_values['redirect']);

Because drupal_get_normal_path deletes the language from the path.

function path_redirect_init:

Duplicated the complete content of the function. For the first part I´m now using $GLOBALS['HTTP_GET_VARS']['q'] for $query as this one contains the language prefix. I know this is a dirty hack, because the code is really redundant ;-)

Greats from Germany
LeisureLarry

Interesting. I also had to deal with that issue in path_redirect_init when trying to get path_redirect working with Drupal 6. I ended up doing this:

  if (isset($_SERVER['QUERY_STRING'])) {
    $query = preg_replace('/^q=/', '', $_SERVER['QUERY_STRING']);
  } 

But it looks like that's not as clean as what you're doing. The basic problem was that $_GET['q'] had already been translated, and so it would never match anything I had entered as a "from" when creating a redirect.

If anyone has any idea what's going on with that, I'm all ears!

It looks like drupal_get_path_alias is problematic, too. Check out this patch. It seems to work better all around for me, but it makes me wonder what the motivation for calling that function was.

The more I look at this, the more critical it appears. Or at least it's critical that I understand why it was implemented the way it was. :)

Say I have an alias foo/bar set for node/314. Now I'm going to use path_redirect to redirect baz/quux (the path) to foo/bar (the redirect). What actually gets stored in the database for redirect is node/314, not foo/bar. (Can someone else please confirm this behavior?) This is accomplished via a call to drupal_get_normal_path in path_redirect_edit_validate.

This seems very intentional, looking at the code, since the admin pages pass path and redirect through drupal_get_path_alias before ever displaying it. You'd never know that something different was actually stored in the database.

I think this is problematic, though. What if I change foo/bar to be an alias for node/159 instead? Now my path_redirect will not take me to the correct place, but rather the old place, since node/314 is what was stored.

Thoughts? If we do change this behavior, do we need to do anything to "upgrade" existing installs? Pass all of the redirects through drupal_get_path_alias and then save that back to the database?

For what it's worth, 'm pretty sure that changing this behavior will also fix a number of bugs related to internationalization and the issue(s?) that hass brought up.

I need to reroll this patch against the latest dev version, which includes a number of other fixes, and I also need to fix a couple of issues with the patch itself. In particular, this line:

$form_values['redirect'] = drupal_get_normal_path($form_values['redirect']);

...and the corresponding line for $f_v['path'] should be removed.

Rerolled against the latest dev version. It's working well for me, but it would be nice to see at least a few more people test it out.

Hi,

the patch is not working for me, because $GLOBALS['HTTP_GET_VARS']['q'] is not populated.
Please use $_GET, because $HTTP_GET_VARS is deprecated and will be removed with PHP 6.

- if (isset($_GET['q'])) {
-    $query = $_GET['q'];
+  if (isset($GLOBALS['HTTP_GET_VARS']['q'])) {
+    $query = $GLOBALS['HTTP_GET_VARS']['q'];

Alrighty, I suspected that would probably be an issue. Using $_GET['q'] is not going to work, though, for reasons I think I have spelled out above. Perhaps I need to seek help in the development forum at this point.

Here is an updated patch that should work against the current head of the 5.x branch which also attempts to address Andreas's concern in #13. I'm using $_SERVER['QUERY_STRING'] now instead. I've tested this against MySQL and Postgres with and without clean URLs, PHP 5.2.5, Apache 2.2.6.

Will committing this help get people to test it? Maybe changing the title will? :)

I will test this a bit more but probably commit it soon (minutes or hours). If you're using path aliases, you will probably notice that what you see in the admin interface for path_redirect isn't exactly the same as it used to be. Again, this is because path_redirect wasn't actually showing you the path that it had stored in the database.

I realize this may cause some confusion, but I think it will prevent more serious confusion in the long run, and it will fix a bunch of bugs in the short term.

I went ahead and committed this. It's been working great for me this whole time; I hope others have the same experience!