Merge global redirect functions into Redirect module

Hrm, you can describe more about what's going wrong? I can't duplicate this currently.

No response in over a month, so marking as fixed.

Hey, I got the same issue.
I enter URLs to redirect from and redirect to and press "Save".
I got moved immediately to correct redirect page (the one in Redirect To field) but when I go back to the URL Redirects page I can see no redirect entries at all. The list is empty.

I still cannot duplicate this. Can you please post the details on the redirect you were trying to save?

Redirect From = relative old path, pointing to a non-existent page, a remnant of an old site, for example:
texts/text.html
Redirect To = relative path to actual page, for example:
articles/text
When I press Save, I got redirected immediately to articles/text page (instead of staying in URL Redirects settings page).
If I move back to admin/config/search/redirect, I cannot see any entries. The redirect table in site DB is empty as well.

I do not have Global redirect or Path redirect modules installed.

Ok, I got it figured out.

You cannot create URL Redirect if the Redirect To path is a synonym OR has a synonym defined.
You won't receive any warning about that, you just got moved silently to the target web page.

Further info:

the problem is with

// Normalize the path.
  $value = drupal_get_normal_path($value, $form_state['values']['language']);

part of the code (redirect.admin.inc, line 363). When drupal_get_normal_path got invoked, it receives a "normalized", i.e. "non-synonym" path as a $value.
Then drupal_get_normal_path itself performs a hook_url_inbound_alter(), so Redirect module receives the priority again and redirects us to the "synonym" page asap.
It means the code after line 363 is never get executed IF the path has a synonym defined.

sub

No, that patch does not fix this issue.
I have tested the last dev. version with that patch incorporated, but the reason of this erroneous behaviour I have already explained in the above posts.
Actually escalating this to Critical, because the main function of the module is broken - and I think like 90% of the sites use path aliases.
I will give it a good debugging again when time allows and MAYBE I will be able to come up with the solution. Still, want to hear from Dave if he was able to reproduce this issue.

Yeah I confirmed this. I'll have to disable the global redirection functionality temporarily until I can figure out a solution.

I am experiencing this issue as well.

subscribing

I just committed the temporary disabling of global redirection until I can work out a better solution. It may have to wait until after 7.0 release.

Please, take a quick look at this part of code (redirect.admin.inc, line 356 onwards):

function redirect_element_validate_redirect($element, &$form_state) {
//  $value = &$element['#value'];                                                         <== possibly disabling this
  _redirect_extract_url_options($element, $form_state);
  $value = &$form_state['values']['redirect'];


  // Normalize the path.
//  $value = drupal_get_normal_path($value, $form_state['values']['language']);    <<== and this

  if (!valid_url($value) && !valid_url($value, TRUE) && $value != '<front>' && $value != '') {
    form_error($element, t('The redirect path %value is not valid.', array('%value' => $value)));
  }

  return $element;
}

I have disabled these two lines of code and everything seems to be working as designed.
The reason to disable the drupal_get_normal_path call is this: you have already called _redirect_extract_url_options (line 358), which performs drupal_get_normal_path all right (line 404), so the $value (line 359) contains normalized path. When you call drupal_get_normal_path for the second time, you got redirected immediately via hook call because you are not supposed to pass normalized paths to drupal_get_normal_path function!

So I would just roll back the last change (enabling global redirect again) and change just these two lines (commenting or deleting them) to fix this issue. Works perfectly for me. At least I can not see any possible flaws in this solution.

P.S. Plus one more minor fix: line 378

  $options = & $form_state['values']["{$type}_options"];

should not contain a space between "&" and "$" symbols.

Changing scope and priority

Here's a patch that

a) does what Azol describes in #16
b) renables the global redirect functionality. I also had to change the $redirect variable to a stdClass instead of an array because that gave a lot of notices

Global redirect seems to be working fine now (so node/1 to 'a-nice-alias') and managing redirects in the admin too.

Needs work, still have the problem of editing an existing alias, the value never gets saved, will look into it in a few moments.

Ok temporarily a patch which makes editing redirects or aliases work. Removing the code as suggested in #16 is not a solution, so that code is back in. Path.admin.inc also calls drupal_get_normal_path() in its validate function so you get redirected when editing url aliases.

I've added (a very ugly) check in redirect_url_inbound_alter() checking on arg(2) and arg(3) for 'config' and 'redirect' or 'path'. This works for now so we can edit which is good, however, this can never be RTBC.

I think we should find another way than using redirect_url_inbound_alter() because as soon as someone calls drupal_get_normal_path() you might get redirected without knowing what's going on. @Dave Reid If you have suggestions, add some comments or ping me on IRC, I'd be happy to try and test other things out for this patch.

What setup do you have? #16 worked for me so far...

Azol, try editing an existing URL alias (at admin/config/search/path), you'll be redirected too and no save.

sub

Subscribing

sub

Patch from #20, corrected, simplified, and also avoids redirecting from cron.php, xmlrpc.php, update.php, etc.

EDIT: Also filed bug against core: #1217668: drupal_path_initialize() should not call drupal_get_normal_path() when run from cron.php.

Additional fix to avoid php 5.3 errors from implicitly casting an array to an object.

Trying again, with the correct patch file this time.

Re-rolling patch from #30, removing extra tabs/spaces from two line endings.

Revised patch also prevents the redirect from running under drush and other CLI applications.

Improved patch re-enables the relevant admin settings and also leverages the existing redirect_can_redirect() function.

I patched with #32, but got the following error when visiting /index.php:

Recoverable fatal error: Argument 1 passed to redirect_object_prepare() must be an instance of stdClass, array given, called in drupal/sites/all/modules/redirect/redirect.module on line 930 and defined in redirect_object_prepare() (line 649 of drupal/sites/all/modules/redirect/redirect.module).

Updated patch.

Fixed typo.

Trying again...

(sigh) I'll get it right eventually...

I don't think it's quite working the way intended.

• I've added this patch to a local install.
• I've enabled all of the GlobalRedirect options.
• I have a Panel page called "homepage" and the "site_frontpage" variable is set to "homepage".
• When I visit /homepage it does not redirect to the homepage, like it would with GlobalRedirect.

Does what I'm experiencing mean that the issue needs to go back to "Needs work", or that I need to submit a new issue?

Reviewing the configuration I realized that this was a new feature, so I added a new issue: #1255998: Option to redirect to the site root directory if the path == site_frontpage

DaveReid, what re-engineering were you intending before re-adding this functionality?

Works for me. I'm not that experienced with this module, so I don't want to set this to RTBC. Anyone else?

#39: redirect-restore-global-redirect-905914-39.patch queued for re-testing.

args() is being crushed by hook_url_inbound_alter which means any code that relies on it is falling down. My fix is just to totally remove redirect_url_inbound_alter. I'm going to roll with this until something better comes along.

args() is being crushed by hook_url_inbound_alter

Please cite a reference or otherwise support your assertion. I see nothing in the code you remove that changes the output of the arg() function.

Furthermore, you are hijacking this issue with an unrelated patch which has nothing to do with the problem and fix addressed by this issue. Please open a separate issue, instead.

Re-uploading the working patch which is currently under review.

#47: redirect-restore-global-redirect-905914-39.patch queued for re-testing.

Anyone else running into problems with canonical URL support clashing with the pager and throwing the browser into an infinite loop?

@DamienMcKenna -- Please describe how to reproduce this error so we can write a test case and fix it.

I'm not really sure what I am reviewing here, I was hoping this patch would add the functionality of global_redirect module into redirect, but after testing I think maybe that's not the case - so I'm leaving as needs review.

Results of testing: navigating to node/15 did not gently redirect me to the drupal_get_path_alias() version of the path. My page still loads and the URL says node/15. Similarly, navigating to index.php?q=node/15 also did not redirect to the clean-urls version of the path :(

For now, I'm still going to use both modules, but it would ne nice if we could get all the old D6 redirect modules into Redirect :)

@pillarsdotnet: I'm using a Panels node view for the content type, on the node I have a view with a pager, clicking on any page in the pages list should go to e.g. /mypage/page/1/0 but instead is redirected back to /mypage. I haven't dug into it too much, but disabling the "redirect canonical URL" option fixes it.

@jenlampton: Did you enable the "redirect canonical URL" option? What options did you enable in the Redirect settings page?

@DamienMcKenna I am running the dev snapshot from Dec 2, and *thought* I had applied this patch. It appears 'git apply' didn't work at all for me, and after you asked me if I'd turned on canonical redirection, I realised that I'd never even seen that option! :) a patch -p1 did the trick for me this time, but bad news...

With the following settings, I get this error when visiting index.php?q=node/15:

Recoverable fatal error: Argument 1 passed to redirect_redirect() must be an instance of stdClass, array given, called in .../sites/all/modules/contrib/redirect/redirect.module on line 287 and defined in redirect_redirect() (line 916 of .../sites/all/modules/contrib/redirect/redirect.module).

Redirect from paths like index.php and /node to the root directory. (Checked)
Redirect from non-clean URLs to clean URLs. (Checked)
Redirect from non-canonical URLs to the canonical URLs. (Checked)
Remove trailing slashes from paths. (Checked)
Allow redirections on admin paths. (Un-Checked)

The last patch was not applying for me. Attached is a re-roll. I've yet to test the actual workings of the patch.

Working on the patch from #56 (which applies cleanly) :)

- changing pathauto "patterns" and re-saving node with pathautp checkbox checked redirects.
- changing node title and re-saving node redirects.
- visiting the Drupal system path for the node (node/1) redirects.
- visting non-clean url (?q=node/1) redirects.
- visiting actual full url (index.php?q=node/1) also redirects
- visiting any one of the paths above with a trailing slash, redirects.
- visiting the Drupal system path with a query string (node/1?foo=bar)redirects, retaining the query string.

Anything else I should be testing? test bot?

@jenlampton: did you try a pager? E.g. attach a view to a page and have sufficient records in the view that it makes a pager display, the URLs will be in the form /clean/url/page/0/1 but they all redirect back to /clean/url.

A page-view pager seems to be in the format pager-test?page=1. A block-view pager also adds a query string articles/test-article?page=4.
... still not sure where the redirects come in for views.

But Test bot likes it :)

#1255998: Option to redirect to the site root directory if the path == site_frontpage

Given that the issue above has been marked a duplicate of this issue, I imagine that after applying this patch, the path I have specified as my front page should be redirected to the to the sites base URL, but this is not the case.

Steps to reproduce:

1. Create a view with a page display with the path "front"
2. Set the front page path to "front"
3. BUG: example.com/front is not redirected to example.com.

So in other words with this patch Redirect module should be replace for Global Redirect module ?

I'm running into the same problem with front page, but all other redirections are working. Changing status.

@batigol, yes :)

I have added global home page redirect for both clean and "dirty" URLs for AntoineSolutions and myself.
It's reworked patch from #56 and it should do all things from #57 and a bit more
* example.com/index.php?foo=bar&q=node/123 -> example.com/node/123?foo=bar
* example.com/index.php?q=node/123&foo=bar -> example.com/node/123?foo=bar
* example.com/index.ph?q=node -> example.com/ (if "node" is home page)
* example.com/node -> example.com/

patch fixed (i hope)

I don't use front page on my site so I can't test it. But it seems to work fine on my friend page.

However do I need to install Global Redirect in addition to this module ? Does redirect works the same way or there are some differences ?

@pwinlacki -- With the patch, the Redirect module should do everything that Global Redirect does, and more.

Pillarsdotnet thank you for clearing this out. I can't wait till beta5/rc1 release !

I would note one specific thing: You should not set alias (welcome) for front-page path (node/1). If you do this redirect will send you to front page's alias (http://example.com/welcome) if you type site URL (http://example.com)
Is this a bug to resolve?

@fasdalf@fasdalf.ru -- You should not RTBC your own patch. Also,

+  $is_query = (strpos($request_uri, '?q=') !== FALSE) || (strpos($request_uri, '&q=') !== FALSE);

Yuck! Isn't there anything in the Drupal API to accomplish this?

New patch checks if requested page is front and skips redirect to alias.

Patch rebuilt

Sorry. Can't figure out what it's don't like. Attach is new version of home/front page feature.

After I figured out what you changed in the patch file, I applied the same changes to my own copy of redirect. I do not see the site redirecting from 'welcome' to the front page with no path (to use your example).

Trying a new approach.

Hm, I'm not sure why that test failed.

I disabled cleanURLs and un-checked the cleanURL redirect checkbox, and my site redirects from ?q=redirect to ?q=node just fine.

Rerolled with one other minor bug fixed, but if the test needs to be rewritten I may need some help with that.

IMHO there is a problem with this code

+  // Redirect to front page.
+  if (variable_get('redirect_global_home', TRUE) && drupal_is_front_page()) {
+    $redirect_global = TRUE;
+    $request_uri = '';

It will make endless redirect. I believe it should be like this:

+  // Redirect to front page.
+  if (variable_get('redirect_global_home', TRUE) && drupal_is_front_page() && isset{$_GET['q']}) {
+    $redirect_global = TRUE;
+    $request_uri = '';

@fasdalf: I believe you meant:

+  if (variable_get('redirect_global_home', TRUE) && drupal_is_front_page() && isset($_GET['q'])) {

Also see http://drupal.org/node/1183208

This is slightly related. The stub for implementing global redirect functionality is causing problems with some modules: #1463010: Entity_load during url_inbound_alter is incompatible with text fields

in response to #85,
isset($_GET['q'])) doesn't prevent the infinite redirect on the home page either. Would love suggestions as to how to get that working.

Okay, here I tried to determine if we were on the front page already by comparing $_SERVER['REQUEST_URI'] to base_path().
Also trying a new approach, this time with the redirect firing in hook_url_inbound_alter instead of in hook_init.

and one more time sans dsm.
dunno why tests are failing, but maybe someone else can help with that?

Please note that calling entity_load() can break things if done before menu_get_custom_theme().

See #1370964: Redirect interferes with hook_custom_theme for details.

This patch results in infinite redirections when Persistent URL (purl) module is installed. Investigating.

Doing a redirect from hook_url_inbound_alter is not right; this hook is called from drupal_get_normal_path(), and callers of drupal_get_normal_path() certainly do not expect a redirect to be done.

IMHO all redirects should be done in hook_init().

#91: enable_global_redirections_with_frontpage-905914-91.patch queued for re-testing.

Ok, so the errors are because the test expects the user to be redirected to the actual redirect url, but it might (should) redirect to the frontpage (no internal path) if the redirect url is actually the frontpage. So I guess we have to fix the test.

There's some logic already available at redirect_url_inbou'nd_alter checking for the redirect_global_home variable. And I don't think it should default to TRUE, since that wasn't the behaviour before. I'm just changing that and hoping it fixes the tests until we improve them.

Marked #1793330: Global redirect feature is not working at all as a duplicate.

Re-rolled against current 7.x-1.x checkout.

Missed a closing brace; sorry.

Not sure whether the static is supposed to store a single redirect or an array as returned from (e.g.) redirect_load_by_source().

Going with the latter for now...

Patch works great for me!

Removed whitespace errors too.

Can someone update the issue summary? The patch is RTBC and passing tests, but I don't remember what it's supposed to do. At a camp this weekend so might have time to commit this.

Tagging.

I tried to update the issue summary based on reading many of the comments here (and skimming some). I didn't review the code.

reverting unintentional de-tagging...

Update: Patch still applies cleanly, and still works :)

Having got rid of the module globalredirect recently and added redirect, I did miss the functionality that globalredirect provided around sending 'node/1234' => path_alias.

I applied this patch to 7.x-1.x-dev, and it mostly seemed fine. The only major issue is that the homepage no longer is accessible on '/', as it redirects to 'home' (the path alias for node/1). This some how causes panels selection criteria to break (page is the site front page).

Obviously there is some work around the homepage that needs to be completed before this is ready to go.

I started altering the patch, and then worked out it would probably be easier if I merely implemented hook_init() myself and do the redirect for node/1234 => path_alias

One of the reasons for implementing global redirect is to avoid having two urls point to the same content. If the example.com front page is set to home which is an alias for node/1 then exactly one of the two rules should be enforced:

1. Visiting http://example.com/node/1 or http://example.com/
   ... should redirect to http://example.com/home
2. Visiting http://example.com/node/1 or http://example.com/home
   ... should redirect to http://example.com/

Do your panels selection criteria only break when rule #1 is active, or do they also break when visiting http://example.com/node/1 or http://example.com/home directly? If the latter, then I'd say you have identified a bug in panels, not in the redirect module.

When I had the globalredirect module enabled, node/1 would go to '/' which was perfect as it was the homepage.

With this patch applied, this is what happened:

1) Visit '/', redirected to '/home', panels selection criteria fail
2) Visit '/node/1'. redirected to '/home', panels selection criteria fail

I don't know if it makes any difference, but the site I am working on is multilingual as well (so there is indeed two site_frontpage)

@pillarsdotnet i think the expected result is option #2: both node/1 and /home should redirect to /

I think that's what @wiifm says in #116 and that's what I'd expect as well. I think (from #98) that there's a variable (redirect_global_home) and a test for that.

I'm putting this patch on yet another site, here is what I did:

1) enable redirect module (dev version, with patch applied)
2) go to redirect settings page to enable 'global redirect' functionalty
3) enable the setting "Redirect from paths like index.php and /node to the root directory."

And here are my tests:

Navigate to the home page '/' - expect no redirection. Works as expected.
Navigate to my panel for use on the front page '/frontpage' - expect redirect to '/' - Works as expected.

changing status, I have a feeling you just forgot to check the box to turn on the home page redirect feature. I can't get it to misbehave. :)

Reroll.

Minor change

+    'redirect_global_home' => FALSE,
+    'redirect_global_home' => 0,

and

+    '#default_value' => variable_get('redirect_global_home', FALSE),
+    '#default_value' => variable_get('redirect_global_home', 0),

Another style error :(

However, this doesn't completely work as expected for pages with aliases.

Enable Path & Redirect
Create a node and set alias to home
Check redirect_global_home
Set Site Information Default front page to home

node/1 redirects to home

Set Site Information Default front page to node/1

node/1 redirects to home

A quick look shows that on /home, $_GET['q'] is set to the alias when drupal_is_front_page() is called.

No time to investigate further, but the quickest workaround I saw was to remove the alias, and to use Page manager / Panels to embed the node on a custom page with the URL home and setting this as the default front page in the site Information settings.

I notice that this patch includes new functionality, but doesn't include any tests.

The discussion in #117/119 makes it clear that some tests could be helpful to be more confident that the functionality works as expected.

Changing status as per Greg's comment and issue related to path aliases noted in #116 & #121.

The functionality I'm looking for is the "Deslash" setting from Global Redirect that 301's a request for about-us/ to about-us

I've applied #121 to latest dev of Redirect and it looks like the setting redirect_global_deslash (Remove trailing slashes from paths) doesn't Deslash anything and the setting isn't being referenced anywhere.

The work being done here is great, and is moving in the right direction. Although it does what it is supposed to, I've discovered a conflict that it produces. I am using the Video Embed Field module to display youtube videos in a colorbox on my site. With the latest version of the patch applied, a $variables array created by Video Embed Field is not populated correctly when using a colorbox to load the video. As a result, the video does not display. I originally opened an issue over there at #1908058: $variables array not being built correctly when using colorbox..

Not sure where the conflict is exactly, but I figured I should point it out. Let me know if you need more clarification.

Only thing holding me back from making the jump to this module is the entity id url -> alias redirection. IMO that is one of the best features of globalredirect, but I need my clients to have an admin interface for redirects. Any ideas of when this feature will come in?

After reading this post as well as several others in the queues about the differences between Global Redirect and Redirect, I too am confused as to a few things. First, Obviously we want the functionality of cleaning up duplicate content that Global Redirect provided.

Also, in Pathauto config screen under settings recommends using the "Redirect" module when "leaving old aliases intact when generating new aliases." Which, is of course what I want as I am about to do a massive url structure rewrite and want redirects from the old aliases.

So, Can I use both Global Redirect and Redirect despite the effort of merging the former into the latter? Are their conflicts?

Secondly, as of the patch and few comments above, this still needs testing and people are finding minor bugs.

So then, what is the stable solution? Do / Can I use both Global Redirect and Redirect together? Or is the merging functionality stable enough that it has made GR obsolete, and I can happily get rid of it and use just Redirect as the complete solution?

Thanks!

I'm currently running an older dev release of Redirect that contains the alias redirecting from the system paths which I needed. I disabled globalredirect and enabled that and it works a treat. I don't think it'd be a good idea to run the 2 side by side though.

Here's where I'm at working on #121:

* Got Deslash (Remove trailing slashes from paths) working.
New features:
* redirect_global_index setting to remove 'index.php' from non-front paths.
* redirect_global_canonical_front setting to indicate if you want the front page to redirect to its canonical URL. I found that with the existing redirect_global_canonical setting, a request for the root "/" would redirect to "/welcome-to-my-site" because that is the front page canonical URL. That's undesired behavior in most cases.

I've tested this with pathauto and subpathauto with success. Obviously it still lacks test cases.

The big structure question I have is should this global behavior reside in redirect_url_inbound_alter() or redirect_init()? I basically left it as it was - calls to redirect_redirect() split between the two. I'm guessing redirect_url_inbound_alter() should only contain calls to redirect_set_current_redirect() and no calls to redirect_redirect() so that other modules can still execute their hook_url_inbound_alter(). Then redirect_init() can use redirect_get_current_redirect() and do the redirect.

Here is #129 + Add Slash functionality for use with the Trailing Slash module to enforce trailing slashes.

I used #130 for a week, works for me so far. Thanks.

Does this need anything other than tests?

Patch #130 is giving me redirect loops with the "Redirect from non-canonical URLs to the canonical URLs" option turned on.

# curl -I http://dev.papercutmag.com/articles/2013/06/24/david-sheldrick
HTTP/1.1 301 Moved Permanently
Date: Thu, 27 Jun 2013 04:50:59 GMT
Server: Apache/2.2.24 (Amazon)
X-Powered-By: PHP/5.3.25
X-Drupal-Cache: MISS
Expires: Sun, 19 Nov 1978 05:00:00 GMT
Last-Modified: Thu, 27 Jun 2013 04:50:59 +0000
Cache-Control: no-cache, must-revalidate, post-check=0, pre-check=0
ETag: "1372308659"
Location: http://dev.papercutmag.com/articles/2013/06/24/david-sheldrick
Connection: close
Content-Type: text/html; charset=UTF-8

How do you redirect from sub.domain.com to domain.com/sub ?

I'm also getting the following error when I have a view on my home page with a pager, and I click any of the pager links:

User warning: Infinite redirect loop prevented. in redirect_redirect() (line 1040 of drupal-7/sites/all/modules/contrib/redirect/redirect.module).

I don't have any redirects set up for my home page, but it is a panel that's set up at /front. If I UNcheck the box for Redirect alternative front page URLs to the root directory, then everything behaves properly.

any news on redirects? thanks for your hard work :)

I guess nobody is working at it at the moment. All focus is on Drupal 8.

@Corwin I believe that sub domain redirects need to be done at a webserver level rather than through Drupal.

@acbramley there is always the case when webmaster has limited access to webserver configuration - so it may be an option in some Drupal module, but now I am thinking, if it is even possible...

What features does redirect currently have, in the stable release which are from global redirect, which were not included previously in the D6 path_redirect module?

For me, the "basic" functionality of global_redirect was two things, removing trailing slash and redirecting node/123 to path alias. It appears the stable version of redirect currently does neither of these.

Because of this, I would recommend changing the description of this project because it's confusing for an experienced user of Drupal, who have used the previous modules this module claims to replace. For inexperienced users, I assume it would be worse.

I would recommend stating what features the stable version provides (or dev) from Global Redirect, and which features it does not implement. Then I would recommend linking to this patch and including what features this patch includes from Global Redirect, which work (and link to a specific patch version).

Perhaps this would get more people in here testing and moving this forward. This patch issue has been ongoing for 3 years now.

--

This is just my experience from reading over the description of this module, thinking to myself, awesome I don't need global redirect...only to find out I still do.

Moving this up in priority, just in case it helps. ;)

#121 and #129 patches not working. I am getting infinite loop messages in certain pages.

I have tested it again, and there are still problems. Installing DA and i18n give me loops almost all the time but i guess integration with those module will be add later on.

attempting to update the summary

Here's an update that should correct the redirect issue described in #136, #143, etc. I was not accounting for query strings.

The big outstanding thing is tests.

I've been using this in production successfully since March. The one minor bump in the road for me has been using it on a site that is behind an Apache ProxyPass setup; redirect_can_redirect() returns false since $_SERVER['SCRIPT_NAME'] != $GLOBALS['base_path'] . 'index.php'

Setting to Needs review so the testbot will test the most recent patch.

I'm marking this issue RTBC since the patch in #145 is working great for me. If tests are needed before this can be committed feel free to push back, but I'd love to get this in sooner rather than later. Almost all my sites are using one version of this patch or another ;)

I just ran into an infinite redirect loop on a multilingual site for the case where the redirect_global_home = 1, and then visiting http://example.com/en.

I believe globalredirect used to deal with this (as well as redirecting http://example.com to http://example.com/en). I'm sorry I don't have time to provide a patch right now, but am currently accounting for this in hook_redirect_alter().

if ((current_path() == variable_get('site_frontpage')) && (in_array(request_path(), array_keys(language_list())))) {
    $redirect->redirect = FALSE;
}

@ezeedub, have you applied patch from this issue first #1796596: Fix and prevent circular redirects?

@ezeedub, I restore this issue status.

kalabro, do you recomend to apply patch #146 on https://drupal.org/comment/8506117#comment-8506117 and then this. there is no conflict? sorry, i am learning about patches and if they run on the same dev, won't they interference? thanks :)

@candelas, I've applied both on my local environment and after testing deployed to live website.

thanks kalabro. I have installed it and until know it looks ok. i will report if i have any problem :)

@kalabro You are right, I had not applied that other patch. I was confused about whether this issue superseded that one or not. Applying fixes the issue I mentioned in #148.

However, I got to that issue via another. Using globalredirect, paths such as /user were redirecting to /en/user. I switched to redirect though because of the infinite looping issue. Which brought me here (in addition to the other issue you just referenced).

So, long story short... I've tracked what I think we really need, and that is to re-add the globalredirect functionality of prepending the language prefix.

Here's another try which handles language prefixes better.

Argh, add new language prefix to redirect_variables()...

157: redirect-global-905914-157.patch queued for re-testing.

Add test for locale in redirect_redirect()...

I think there may be a problem in #161 with language.inc being conditionally loaded independent of whether or not the locale module is enabled.

Specifically, I'm getting...
Fatal error: Call to undefined function language_url_split_prefix() in [snip]/redirect/redirect.module on line 1125

I actually have a few other patches applied so for me that's here.

/**
 * Redirect callback; perform an URL redirect.
 */
function redirect_goto($redirect) {
  $redirect->redirect_options['absolute'] = TRUE;
  // Prevent a path like 'index.php?q=node/1' from redirecting to '?q=path-alias'
  // if canonical redirection is disabled. This will make url() treat 'node/1'
  // as if it is already an alias and prevent a drupal_get_path_alias() lookup.
  if (!variable_get('redirect_global_canonical', 0)) {
    $redirect->redirect_options['alias'] = TRUE;
  }
  if (module_exists('locale')) {
    // Peel off language prefix if already there.
    list($language, $raw_path) = language_url_split_prefix($redirect->redirect, language_list()); # <------ Error is here

There's a long issue in pathologic regarding a similar problem.
https://drupal.org/node/1848554

Their solution was to do this...

 // Let's see if we can split off a language prefix from the path.
 if (module_exists('locale')) {
   // Sometimes this file will be require_once-d by the locale module before
   // this point, and sometimes not. We require_once it ourselves to be sure.
   require_once DRUPAL_ROOT . '/includes/language.inc';
   list($language_obj, $path) = language_url_split_prefix($parts['path'], language_list());

If I look at locale.module, I see that file being included on demand a lot so it seems like module_exists is probably not quite enough.

grep -irn language.inc locale.module
locale.module:509:        require_once DRUPAL_ROOT . '/includes/language.inc';
locale.module:639:  include_once DRUPAL_ROOT . '/includes/language.inc';
locale.module:1019:  include_once DRUPAL_ROOT . '/includes/language.inc';
locale.module:1069:      include_once DRUPAL_ROOT . '/includes/language.inc';

Attached is a patch that adds a require for the language.inc

There is redirect loop for "Redirect from non-canonical URLs to the canonical URLs"
Clean URLS are also getting redirected and enters redirect loop - which is finally detected and stopped
inside redirect_redirect()

Disabling "Redirect from non-canonical URLs to the canonical URLs" from settings,
"Redirect from non-clean URLs to clean URLs" then has no effect and does not redirect no-clean to clean.

Attached patch is based on #161 and adds whitelist support to the language redirect option.

It is quite hard to understand this issue status. What blocks commiting of finished features? What features are not finished yet?

I think this information should be in the issue summary for issues long like this.

changing status for testbot.

This is a reroll of 166 to apply to 7.x-1.x-dev. The only difference is the removal of the one failing hunk. As far as I can tell, this was fixed a different way in dev.

@@ -979,7 +1042,7 @@ function redirect_purge_inactive_redirects(array $types = array('redirect'), $in
  *
  * @ingroup redirect_api
  */
-function redirect_redirect($redirect = NULL) {
+function redirect_redirect(stdClass $redirect = NULL) {
   // First check if we're in an infinite loop.
   $session_id = session_id();
   if (flood_is_allowed('redirection', 5, 15, $session_id ? $session_id : NULL)) {

Same as patch in 170, except a variable was given the wrong default value (causing weirdness for front page paths). See interdiff.

Now, given the following 3 nodes with the following aliases:

node/1 => foo
node/2 => bar
node/3 => home (set as site home page)

Enable the module. Visiting all paths now redirect to their aliases.
Edit: the 'node/3' path now redirects to '/' (home).

When the "Redirect the front page to its canonical URL" is checked, www.example.com/node/3 is directed to "home" AND www.example.com/ is also directed to "home".

@jessehs: your patch does not take care of #162, so it does not work on multilingual sites.

Changing status based on #172.

Added #162 require_once() to #171

Fixed a small typo in the issue summary, an open LI was missing.

Re-rolled patch from #174 against latest 7.x-1.x (7.x-1.0-rc1+13-dev).

Patch #178 FAILS when applied to the latest dev (2015-Jan-05).

patching file redirect.admin.inc
patching file redirect.module
Hunk #6 FAILED at 1215.
1 out of 7 hunks FAILED -- saving rejects to file redirect.module.rej

Applied fine to the tip of the 7.x-1.x branch for me using either "git apply merge-globalredirect--905914-178.patch" or "patch -p1 < merge-globalredirect--905914-178.patch"

Tried again and got the same error with Mac OS Terminal with these:

patch < merge-globalredirect--905914-178.patch
AND
patch -p1 < merge-globalredirect--905914-178.patch

Both "git apply" and "patch" commands worked for me, latest 7.x-1.x.

@deanflory I mentioned that it applies cleanly to the tip of 7.x-1.x branch. If the version you have is something else it might not apply. For example, I get 2 hunk fails on 7.x-1.0-rc1. Ping me (greggles) on irc if you want to discuss applying patches more (probably other people would be happy to help too, especially if your goal in applying it is to provide a patch review).

Just reporting what I'm experiencing with the patch in #178 and the latest dev as mentioned in #179:
Last updated: January 5, 2015 - 16:18
Last packaged version: 7.x-1.0-rc1+13-dev

If it's somehow just me then let's not drown this issue with whatever oddity I'm experiencing. "Move along" :)

I can not apply patch #178 too. This part of patch was'n applied.

--- redirect.module
+++ redirect.module
@@ -1215,7 +1291,7 @@
       // If this is a command line request (Drush, etc), skip processing.
       $can_redirect = FALSE;
     }
-    elseif ((variable_get('maintenance_mode', 0) || defined('MAINTENANCE_MODE')) && !user_access('access site in maintenance mode')) {
+    elseif (variable_get('maintenance_mode', 0) || defined('MAINTENANCE_MODE')) {
       // Do not redirect in offline or maintenance mode.
       $can_redirect = FALSE;
     }

I see it is applied to module already. So, I've just captured patch after #178 was applied without part above.

I test the patch in one of the site I develop , it has around 3k visitor/day with 2 languages and we just migrate from the old system, so I redirect many urls , and the module play nice with the patch, thanks

@Majdi did you use latest #186 patch?

@pwiniacki yes patch #186

Using the patch at #186, when locale is enabled using language path prefixes, viewing the home page in any language other than the default language causes a redirect loop. In redirect_url_inbound_alter(), when redirecting the front page to the root level (starting at line 252 of a patched redirect.module file), there is no check that we are at the root level for a language other than the default. I fixed this by adding a comparison between the current language's prefix and the request path.

Updated patch and interdiff attached.

+++ b/redirect.module
@@ -251,7 +253,8 @@ function redirect_url_inbound_alter(&$path, $original_path, $path_language) {
+      && "/$language->prefix" != $request_path) {

Haven't tested but should this rather be base_path() . $language->prefix != $request_path oslt?

Patch #190 is giving me redirect loops if the option "Redirect from non-canonical URLs to the canonical URLs." is selected. Here's a new patch, which fixes this issue.
It also fixes the comment #191.

The function redirect_load_by_source must return "The first matched URL redirect object", not a list of objects.
Patch #193 fixes this issue. It is now also possible to apply patches from issue #1796596.

D'oh! Forget to change a line in patch #193. Hope this one is better!

I can confirm that the code from the last patch #195 works great.

So this patch makes redirect module (besides some of it's other functions) work as global redirect? I don't need to install global redirect then?

yes no need for global redirect

#195 is working very good. Great work!

EDIT: also applied to rc-3 - working good.

#195 works great! Please commit this.

Dave Reid, you proposed to merge global redirect functionality into redirect module?
We hesitated with going forward with adding this functionality to 8.x branch because this issue was stuck for years.

Should we go ahead and merge our 8.x global redirect functionality into redirect module port and thus deprecate the 8.x port of global redirect? It would be a good moment to do so before Drupal 8 is released.

@miro_dietiker +1 for the idea.

Just one more point , globalredirect has this option "Case Sensitive URL Checking" I think this is missing in this patch

Yes, this is still planned for the 7.x module, so I would say it should go in 8.x. I'm just a bit more focused on trying to get everyone a stable 1.0 release they deserve in Drupal 7 now, before I go back to features again.

Dave Reid, will you make next RC with this patch?

To anyone who didn't quite understand Dave's comment #205, what he meant was that he's not going to add more features to v7.x-1.0, he's focusing on bug fixes so that the final 1.0 release is stable. However, once 1.0 is out he'd consider adding new features, including this one.

That said, Dave, you added the "D7 stable release blocker" tag to this back in comment #48, if you no longer feel that's appropriate could you please remove the tag to help avoid confusion, because you've basically indicated for four years this would be included? Thanks.

DamienMcKenna, Dave added D7 stable release blocker tag, so I confused...