Incompatible with hook_url_inbound_alter()

Your patch includes a file not form global redirect, re-rolling!

@Catch - could you provide an example custom module which uses hook inbound alter?

I have tried the above patch and although it stops the redirect loop, it also exposes duplicate urls. Basically, it's fixing one problem by completely disabling the system. Of course, it's very possible that my custom module is at fault here ;)

I made a module called gr_dummy which was simply this:

define('GR_DUMMY_PREFIX', 'thingy');

function gr_dummy_url_outbound_alter(&$path, &$options, $original_path) {
  _gr_dummy_rewrite($path);
}
function gr_dummy_url_inbound_alter(&$path, $original_path, $path_language) {
  $path = _gr_dummy_remove($path);
}

function _gr_dummy_rewrite(&$path) {
  $alt = _gr_dummy_remove($path);

  if ($alt == $path) {
    $items = empty($path) ? array() : explode('/', $path);
    array_unshift($items, GR_DUMMY_PREFIX);
    $path = implode('/', $items);
  }
}

function _gr_dummy_remove($q) {
  $args = explode('/', $q);

  if (current($args) === GR_DUMMY_PREFIX) {
    array_shift($args);
  }

  return implode('/', $args);
}

(this is mostly "inspired by" purl).

In GlobalRedirect.module, I added this after globalredirect_request_path and before locale_language_url_rewrite_url.

  $original_path = $request_path;
  drupal_alter('url_inbound', $request_path, $original_path, $language->language);
  if (!$request_path != $original_path) {
    return;
  }

Any thoughts?

Could you provide your dummy module which you used to test it please?

(Is this a duplicate of #832862: hook_url_inbound_alter breaks the module?)

An easy example is the D7 port of the purl module:

• http://drupal.org/project/purl
• #1070222: Port PURL to Drupal 7

Enable the globalredirect and the purl module, and all admin/* urls result in an endless loop.

I can confirm that the proposed solution in #2 works for me, and it solves the problem with the purl module mentioned in #6. Patch attached.

Is this a dup of #1101178: Check for outbound url rewrites, or are the inbound and outbound issues separate?

It's not a duplicate, a problem still exists.

Marking #986048: Redirect loop as a dup

Scaling to major, this breaks the module.

Correct me if I'm wrong, but in the patch from #7 shouldn't

(!$request_path != $original_path)

be

($request_path != $original_path)

Another potential issue with patch #7 is that it uses drupal_alter(). The function drupal_get_normal_path() avoids this function so that it can guarantee execution in reverse order of hook_url_outbound_alter(). While unlikely to be an issue for most people, it would probably be better to keep the same execution order.

Any updates on that issue? Anything we could do?

Patch in #7 does stop globalredirect falling into a redirect loop when spaces/purl is enabled, but as noted in #4 it just disables globalredirect and therefore renders it pretty useless.

There is also http://drupal.org/project/redirect

I just opened a new issue #1445934: Fix logic of module, broken when other modules implement prefixes. which as "@Dave Reid" mentions it might be a dup of this. Even if not they are related.

Patch there fixes the issue mentioned here.

Cheers

It looks as if Global Redirect fixed their issue, has this patch been applied to this module?

Is this problem fixed? The http://drupal.org/project/subdomain module is incompatible with global redirect until this problem is fixed.

In my opinion, this is not inherently broken. Implementing modules should also implement hook_url_outbound_alter() so that the URL gets generated the same way with url() — Global Redirect respects this. If you only change it one way, then you will get this problem.

For example, on one client site, I ran into this because we implement a custom language fallback logic and change the internal path served by Drupal if we don't have a translation available for the language and alias requested. This of course confuses Global Redirect; it sees that the alias I'm trying to go to doesn't exist for the language I'm on. I work around this by implementing hook_url_outbound_alter() and hijacking $options['language'] . I change it to our fallback language (which does have an alias) so that when Global Redirect does its alias check, it finds that we are where we should be and leaves us alone.

// Partial snippet
function interstitial_popup_url_outbound_alter(&$path, &$options, $original_path) {
  $paths = drupal_static(__FUNCTION__);
  $static_key = "{$original_path}_{$path}_" . json_encode($options);

  if (!isset($paths[$static_key])) {
    global $language;
    if (!isset($options['language']) || $options['language'] == $language->language) {
      // If they are asking for a URL in the current language, substitute in
      // the parent language. By default it should use our language's base URL
      // anyway.
      $parent_language = i18n_language_load(interstitial_popup_get_object()->language);
      if ($parent_language) {
        $parent_language->domain = $language->domain; // h4x0r
        $options['language'] = $parent_language;
      }
    }
    $paths[$static_key] = array(
      'path' => $path,
      'options' => $options,
    );
  }
 else {
    $path = $paths[$static_key]['path'];
    $options = $paths[$static_key]['options'];
  }
}

Another way to do this is to implement hook_module_implements_alter() and swap out globalredirect_init() for your own custom implementation that does some setup (e.g. changes $_GET['q'] and then calls globalredirect_init() and then fixes $_GET['q'] back). I don't recommend this.

Other than this, the fix should be in the Subdomain module itself. That module knows how best to do it. We cannot possibly cover all cases in Global Redirect without changing the very behavior for which people install the module.

If this was indeed related to #1445934: Fix logic of module, broken when other modules implement prefixes. (and it seems like it would be), this would now actually be a duplicate of #1843500: Wrong usage of prefix in Global Redirect (Patch attached). Seems like a better solution to what we have here.