wp_redirect and canonical redirect do not work as expected on IIS

[peaceablewhale]

To address a bug affecting IIS <=5, wp_redirect use header("Refresh: 0; url=$location") instead of header("Location: $location") on all versions of IIS, which is unexpected.

[peaceablewhale]

This patch depends on #10186. Tested with PHP 5.2.9-2 on IIS 5.1 via FastCGI.

[peaceablewhale]

Patch updated due to #10186.

[westi]

I don't think at the point it time we should be adding fixes to issues in IIS5.

Windows 2000 is already in Extend Support and there are two newer major versions of IIS available.

Closing as WONTFIX

[dd32]

This isnt addressing a bug in IIS5.

WP includes a fix to address IIS5, Which is breaking redirects on IIS > 5

[peaceablewhale]

The existing WordPress code prevents newer IIS to send correct HTTP redirect headers and I am trying to fix that.

[westi]

Sorry. I read the logic the wrong way round.

I still don't think a global server version is correct.

Better to set a $iis_supports_location_header global and use that that use server version in the function.

[peaceablewhale]

How about dropping support to IIS <=5 completely?

[peaceablewhale]

A patch that drops support to IIS <=5 has been uploaded

[ruslany]

I think that dropping support for IIS <= 5 is a good suggestion. Proper PHP support has been added to IIS starting from 5.1.

If support for IIS <=5 is dropped then the patch looks good to me.

[janeforshort]

Most recent patch doesn't show evidence of community testing, and we're in beta, so punting.

[ruslany]

Verified the latest patch on Windows 7 with IIS 7.5. It works as expected.

Note that at the moment there is a bug in IIS FastCGI module in IIS 5.1, IIS 6.0 and IIS 7.0 which causes all redirect calls from PHP to have status code 302. The update for the FastCGI module with the fix for this will be available soon. But I think the patch should be applied anyway, even before the IIS FastCGI bug is fixed. That way WP will start using proper redirects instead of refresh header. Users who have not updated IIS FastCGI will see HTTP 302 redirects (which is still better than refresh header). User who have updated the IIS FastCGI will see HTTP 301 redirects.

[Denis-de-Bernardy]

both patches are plain wrong. they remove fixes for apache/php as cgi.

[ruslany]

Updated patch that preserves the fixes for apache/cgi

[ruslany]

Attached an updated patch that preserves the fixes for apache/cgi.

[peaceablewhale]

Removed the call to status_header()

[peaceablewhale]

As the status code has been passed to the header() function, I don't think we still need to call status_header(). An updated patch has been attached.

[hakre]

Related: #10435

[hakre]

Replying to peaceablewhale:

As the status code has been passed to the header() function, I don't think we still need to call status_header(). An updated patch has been attached.

Reviewed and tested that. Can confirm it. I see no difference here wether or not status_header() is called. Compared the plain headers output as well.

[hakre]

Related: #3528

[peaceablewhale]

Merged with #10435

[hakre]

Reviewed the patch again: I think the check for DOMDocument does the deal for now, the function where this applies to could be referenced. The comment is a bit long to read for one line of code but it does the job. If DOMDocument is replaced later on for compability reasons this code should be refactored again. Looks good so far.

Maybe ruslany can test the merged patch again? Just in case.

[ruslany]

Verified the merged patch. It works correctly.

Also, instead of checking for DOMDocument, the code could call phpversion() and check if it is PHP 5. This could be tracked by a separate ticket perhaps?

[dd32]

This seems acceptable to me.. But I'd like to hear other Devs thoughts on this first.

The proposed solution:

• Remove <= IIS5.0 support
• Move all redirects to use Location header instead of the Refresh header.

[dd32]

This would be better off handled early in a dev cycle however i believe. 3.1 material.

[peaceablewhale]

See also #13059.

[peaceablewhale]

Any update to this issue? Will the patch be checked in?

[peaceablewhale]

Patch updated for WordPress 3.0.

[dd32]

Cross reference to a problem caused by this: #14904 (Install page not redirecting to wp-admin/ on IIS6)

[knutsp]

I have tested 10187-10435-merged.2.patch such:

On IIS6 (PHP5.3, fcgi, no rewrite module) while requesting the root (front page) on an uninstalled blog with a present wp-config.php. Redirects perfectly to wp-admin/install.php

On IIS7 (PHP5.3, fcgi, with rewrite module) for canonical redirects, testing with ?p=id and ?page_id=id. Redirects perfectly to the correct permalink.

[dd32]

Based on #14904, It seems that $is_IIS has been set too early for iis5 environments, and as a result, redirects to the installation page havn't been working properly there in 3.0.

As IIS 5.0 Mainstream & Extended support timeframes from Microsoft ​are over, I feel removing the IIS hacks is appropriate at this time.

[peaceablewhale]

Replying to dd32:

Based on #14904, It seems that $is_IIS has been set too early for iis5 environments, and as a result, redirects to the installation page havn't been working properly there in 3.0.

As IIS 5.0 Mainstream & Extended support timeframes from Microsoft ​are over, I feel removing the IIS hacks is appropriate at this time.

+1 on removing the IIS hacks

[dd32]

(In [15682]) Retire IIS 3,4,5 Set-Cookie redirection workaround. See [2436] for original implementation. See #10187

[peaceablewhale]

It's not necessary to have

if ( php_sapi_name() != 'cgi-fcgi' )

status_header($status);

in the patch, as the status is passed to the header() function.

In addition, [15682] does not fully fix #10187 due to missing fix on canonical redirect.

[dd32]

In addition, [15682] does not fully fix

This ticket isn't quite closed yet, That commit is just the start of fixing the ticket.

It's not necessary to have
if ( php_sapi_name() != 'cgi-fcgi' )
status_header($status);

I'm still confused on that one, it wasn't being applied to IIS which is why I left it out of that initial patch, Until I can work out why it was originally added (ie. the server versions in question) i'm tempted to leave it in there for now.

[nacin]

Dion, what's left here for 3.1?

[dd32]

(In [16797]) Enable canonical redirections for Permalink suporting IIS. Props peaceablewhale See #10187

[dd32]

I'm honestly unsure of what to do with the status_header() call at this stage.

I'm tempted to restore it to the following to be on the safe side for compatibility with other server environments.. This basically restores it to pre-3.1 whilst removing the Refresh hacks for IIS.

if ( !$is_IIS && php_sapi_name() != 'cgi-fcgi' )
		status_header($status)

Reviewing in 3.2 to remove status_header() completely with a move to PHP5 might be the better ground on this one to call the ticket closed for 3.1

[nacin]

iis7_supports_permalinks() is only defined in the admin. That causes an error in redirect_canonical.

[peaceablewhale]

In my patch, I moved iis7_supports_permalinks() to wp-includes/vars.php.

[kcristiano]

Replying to nacin:

iis7_supports_permalinks() is only defined in the admin. That causes an error in redirect_canonical.

I applied peaceablewhale's patch to latest trunk on IIS 7 and IIS 7.5. Works fine. I will attach a patch refreshed off of 16880. (1st attempt to upload a patch, let me know if it is incomplete or flat out wrong)

[kcristiano]

Refreshed to rev 16880

[dd32]

:( I thought I committed that change, But I didn't.
One question I was having at the time, was is vars.php the best location for a function, it is afterall, a vars.php file..

Can anyone think of a better place than functions.php? rewrite.php isnt the best location either

[nacin]

Rewrite.php makes the most sense outside of functions.php. We do have the non-WP rule bits in rewrite, so it's not too much a stretch.

[dd32]

(In [16904]) Fix fatal error on IIS after r16797. props peaceablewhale. Limit variable string searching when possible. See #10187

[dd32]

(In [16905]) Other piece of r16904. Limit variable string searching when possible. See #10187

[dd32]

(In [16907]) Restore pre-3.1 "Status:" header handling for IIS in wp_redirect. Revist in 3.2 to see if it's still required. Fixes #10187