I have endless redirection loops on my IIS sites with the globalredirect module. This is mainly caused by code of globalredirect as it depend on Apache logics not available in IIS. Solving this Apache compatibility issue in core sounds much better to me then in modules. To help globalredirect and other modules depending on this REQUEST_URI I have build the below patches based on the basic concept written at the bottom of http://www.helicontech.com/isapi_rewrite/doc/concept.htm.

CommentFileSizeAuthor
#13 request_uri_IIS_isapi_rewrite_compatibility-D5.patch961 byteshass
#13 request_uri_IIS_isapi_rewrite_compatibility-D6.patch958 byteshass
#13 request_uri_IIS_isapi_rewrite_compatibility-D7.patch946 byteshass
#4 D5_request_uri_IIS_isapi_rewrite_compatibility2.patch937 bytesnicholasthompson
#4 D6_request_uri_IIS_isapi_rewrite_compatibility2.patch933 bytesnicholasthompson
#4 D7_request_uri_IIS_isapi_rewrite_compatibility2.patch922 bytesnicholasthompson
#1 D7_request_uri_IIS_isapi_rewrite_compatibility.patch845 byteshass
D6_request_uri_IIS_isapi_rewrite_compatibility.patch857 byteshass
D5_request_uri_IIS_isapi_rewrite_compatibility.patch861 byteshass

Comments

hass’s picture

hass commented
StatusFileSize
new D7_request_uri_IIS_isapi_rewrite_compatibility.patch845 bytes

And the D7 patch for the tests :-).

hass’s picture

hass commented

I'm not yet sure if we also need to set $_GET['q'] in drupal_init_path() to the same value, but globalredirect works now without endless loops.

Aside, this is a follow up of #166095: IIS <=6 with ISAPI_ Rewrite & clean_url ends with endless loop.

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

Those patches look sensible enough to me...

Just a thought... instead of:

  if (isset($_SERVER['HTTP_X_REWRITE_URL'])) {
    $_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_REWRITE_URL'];
  }

  if (isset($_SERVER['REQUEST_URI'])) {
    $uri = $_SERVER['REQUEST_URI'];
  }
  else {
    ....

Why not just set $uri and do an "elseif" like this...

  if (isset($_SERVER['HTTP_X_REWRITE_URL'])) {
    $uri = $_SERVER['HTTP_X_REWRITE_URL'];
  }
  elseif (isset($_SERVER['REQUEST_URI'])) {
    $uri = $_SERVER['REQUEST_URI'];
  }
  else {
    ....

At least this way, we're not buggering about with the $_SERVER array which *might* cause unexpected behaviour in modules designed to work with IIS, but at the same time we're still returning the correct (aka wanted) value from request_url...

Any thoughts?

nicholasthompson’s picture

nicholasthompson
Primary language English
commented
StatusFileSize
new D7_request_uri_IIS_isapi_rewrite_compatibility2.patch922 bytes
new D6_request_uri_IIS_isapi_rewrite_compatibility2.patch933 bytes
new D5_request_uri_IIS_isapi_rewrite_compatibility2.patch937 bytes

Something like this....

hass’s picture

hass commented

I think we should provide the same value in $_SERVER['REQUEST_URI'] on all webservers if possible. Here with my solution this is possible and we make IIS acting like Apache what is really the best solution.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Needs work

It's really easy to forge a HTTP_X_REWRITE_URL, so this obviously needs some work.

Please also see #298016: No query string added to form action attribute on IIS 7 so some context about how this works in IIS7.

hass’s picture

hass commented
Status: Needs work » Postponed (maintainer needs more info)

Could you please bring some light to us? I have no idea what you are talking about - if the path a user entered in a browser cannot found we give a 404 page back. It is only the called URL you get from this variable. Apache also gives you this URL in $_SERVER['REQUEST_URI']. If there could something be "forged" - it can be on all servers including Apache.

hass’s picture

hass commented
Status: Postponed (maintainer needs more info) » Needs review

Patch in linked issue of #6 looks 100% same like patches in #4. Setting back status.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Needs work

I already noted that it's easy to forge anything in HTTP_X_REWRITE_URL, at least when running a website on Apache. So REQUEST_URL should either takes precedence or a strict condition that checks for ISAPI_Rewrite be added to HTTP_X_REWRITE_URL.

hass’s picture

hass commented
Status: Needs work » Postponed (maintainer needs more info)

@Damien: I think we cannot change the order of the "if" as REQUEST_URL is also set on IIS, but contains something we don't need and that produces the endless loop in globalredirect. So the order need to be as - first check if HTTP_X_REWRITE_URL exists - if not use Apache's REQUEST_URL.

Please provide detailed information how this can be forged.

nicholasthompson’s picture

nicholasthompson
Primary language English
commented

I've googled and cant find anything... In fake googling for the phrase "how to forge HTTP_X_REWRITE_URL" brings this post up as the number 1 result for me ;-)

Claiming that it can easily be forged and yet there being nothing in the google index about it kind of implies an exaggerated claim... Can you enlighten us please.

damien tournoud’s picture

damien tournoud commented
Status: Postponed (maintainer needs more info) » Needs work

@hass, nicholasThompson: the "HTTP_*" values are assigned by PHP using the request headers from the client, without any verification.

It's easy to pass a "X-Rewrite-Url: stuff" value to the server, and this value will be assigned to HTTP_X_REWRITE_URL.

You will need to add server type sniffing if you want to put the HTTP_X_REWRITE_URL test in front.

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new request_uri_IIS_isapi_rewrite_compatibility-D7.patch946 bytes
new request_uri_IIS_isapi_rewrite_compatibility-D6.patch958 bytes
new request_uri_IIS_isapi_rewrite_compatibility-D5.patch961 bytes

New patches attached.

dries’s picture

dries commented
Status: Needs review » Needs work

The patch looks good, and should prevent the forging problem.

1. Looking at the code, I'd still be tempted to switch the if-statements so the 'normal' statement is in the fast code path. Apparently, that is not possible, however, that wasn't clear from the code comments. Might be useful to extend the code comments so that we remember this.

2. 'For more information ...' should not start on a new line.

hass’s picture

hass commented

@Dries: Are you able to provide some feedback if you'd like to follow the Helicon recommendation to set $_SERVER['REQUEST_URI'] (see #1) that would allow us to provide 100% compatibility with Apache on IIS or the variant in #13? I'm not sure what nicholasThompson said about unexpected behaviour with setting $_SERVER variables, but I would like to have the same values in all variables nevertheless it's a $_SERVER variable. I'm not a strong fan of #13.

After we have a clear decision about this I will re-role with your suggestions.

effulgentsia’s picture

effulgentsia commented
Version: 7.x-dev » 8.x-dev

@hass: Is this still stuck on needing feedback from Dries, or has something changed in the last 3 years that should be reflected here first?

Also, if anyone here can help provide info for #1547294: Documentation for request_uri() incorrectly says that only Apache provides $_SERVER['REQUEST_URI'], I'd appreciate it. Thanks.

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
Status: Needs work » Fixed

See https://www.drupal.org/node/1547310#comment-10643716

Status: Fixed » Closed (fixed)

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