i moved my site to a nginx server and i got the 310 (net::ERR_TOO_MANY_REDIRECTS) error, i tried to install the Nginx Fast Config module but the problem is still. Any idea to fix the problem?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DrakeTaylor’s picture

Version: 6.x-1.2 » 7.x-1.x-dev
Assigned: sagitta » Unassigned
Status: Needs work » Active

I can confirm this is also a bug in the latest 7.x-1.x-dev version as well.

DrakeTaylor’s picture

Category: support » bug

Forgot to make this a bug report

DrakeTaylor’s picture

Status: Active » Needs review

Ok, I've found what the issue is (at least in my case). It looks like globalredirect_request_path() is using $_SERVER['REQUEST_URI'] which does not exist in my case. So I added the following fastcgi_param to my conf file for my domain:

fastcgi_param REQUEST_URI $query_string;

Everything works now.

DrakeTaylor’s picture

Status: Needs review » Needs work

Scratch that. It works for global redirect, but causes a ton of other issues.

DrakeTaylor’s picture

Status: Needs work » Needs review

Early testing on modifying the line in #3 to read:

fastcgi_param REQUEST_URI $request_uri;

Is looking good.

paulrooney’s picture

Coder review of globalredirect.module suggests using request_uri()
http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/reques...

Line 375: the use of REQUEST_URI is prone to XSS exploits and does not work on IIS; use request_uri() instead
if (isset($_SERVER['REQUEST_URI'])) {

Line 381: the use of REQUEST_URI is prone to XSS exploits and does not work on IIS; use request_uri() instead
$request_path = strtok($_SERVER['REQUEST_URI'], '?');

Dave Reid’s picture

Status: Needs review » Active

No patch to review here.

DrakeTaylor’s picture

Here's a patch based on the recommendations in #6.

DrakeTaylor’s picture

Status: Active » Needs review

Forgot to change the status

Status: Needs review » Needs work

The last submitted patch, global-redirect-nginx-1317142-8.patch, failed testing.

DrakeTaylor’s picture

Status: Needs work » Needs review
FileSize
939 bytes

Hmm, let's try this again...

nicholasThompson’s picture

Status: Needs review » Fixed

Committed - was looking at changing this earlier but decided not to for some reason. Then saw your patch, changed it and ran the tests to be sure that the green above was true... Seems to be fine! :-)

This has gone into the 7.x branch now.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

malc0mn’s picture

Status: Closed (fixed) » Needs review
FileSize
807 bytes

The patch in #11 still doesn't cut it for me on nginx. The reason for this being the setup of fastcgi:

fastcgi_param REQUEST_URI $request_uri;

This results in this code snippet:

    if (isset($_REQUEST['q'])) {
      $path = $_REQUEST['q'];
    }

to produce a completely wrong $path variable. I can fix this by removing the above mentioned fast_cgi_param, but as it's a common way of setting this up, we should fix the module, not the server. 'q' is always in $_GET anyway...
Patch in attach.

malc0mn’s picture

The more I think about this, the less I understand this globalredirect_request_path() function. Why use the $_GET['q'] or $_REQUEST['q'] at all? I really don't get that, as doing it this way:

/**
 * globalredirect_request_path() is borrowed from request_uri(), but it only ltrim's..
 */
function globalredirect_request_path() {
  if (request_uri()) {
    $request_path = strtok(request_uri(), '?');
    $base_path_len = drupal_strlen(rtrim(dirname($_SERVER['SCRIPT_NAME']), '\/'));
    // Unescape and strip $base_path prefix, leaving q without a leading slash.
    $path = drupal_substr(urldecode($request_path), $base_path_len + 1);
  }
  else {
    // This is the front page.
    $path = '';
  }

  // Under certain conditions Apache's RewriteRule directive prepends the value
  // assigned to $_GET['q'] with a slash. Moreover we can always have a trailing
  // slash in place, hence we need to normalize $_GET['q'].
  $path = ltrim($path, '/');

  return $path;
}

Will cover all cases: clean, non clean etc... Can anyone explain? What use case am I missing then?

eule’s picture

someone will fix this?

dillix’s picture

Is there any news to resolving this issue?

  • Commit eb518e8 on 7.x-1.x, master, 8.x-1.x by nicholasThompson:
    Issue #1317142 - switching ['REQUEST_URI'] to request_uri() to finx some...
Jaypan’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This patch works for us. Can you please commit it.

BarisW’s picture

Looks good to me. thanks for the follow-up patch.

  • BarisW committed e8a1f48 on 7.x-1.x authored by malc0mn
    Issue #1317142 by DrakeTaylor, malc0mn, BarisW, Jaypan: Global Redirect...
BarisW’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x-1.x-dev

  • BarisW committed c996d58 on 7.x-1.x
    Revert "Issue #1317142 by DrakeTaylor, malc0mn, BarisW, Jaypan: Global...
BarisW’s picture

Status: Fixed » Needs work

Sorry, but needed to revert this. On a clean D7 install, with only Global Redirect enabled and this patch - the site ended in an infinite loop on the frontpage. This needs more love before we can commit this.