Clean permalinks and mod_rewrite support for IIS

[man999]

Hello team,

In wp-settings.php you have recently introduced a very important improvement that allows Wordpress clean permalinks to work with the two major url rewriting tools, IIS Mod-Rewrite, and ISAPI_Rewrite.

However the condition

if ( empty( $_SERVER['REQUEST_URI'] ) )

in line 29 does not always work properly. PHP 5.2.x always sets the server variable REQUEST_URI, preventing the execution of the supportive code for IIS url rewriting.

My suggestion is to replace this condition with the following one:

if ( empty( $_SERVER['REQUEST_URI'] ) || substr( $_SERVER['SERVER_SOFTWARE'], 0, 13 ) == 'Microsoft-IIS' )

This condition, will accuratelly recognize IIS. I have tested this on IIS versions 5, 6, and 7, and it works perfect on all of them.

Regards

[hansengel]

A patch of man999's solution—test that REQUEST_URI isn't set or that the current server is indeed IIS

[Denis-de-Bernardy]

PHP 5.2.x always sets the server variable REQUEST_URI

Curious to know what it sets it to. It can't be to something invalid, can it?

[Denis-de-Bernardy]

an empty string based on #100169

[Denis-de-Bernardy]

#10169 even. it also suggests to use $_SERVERHTTP_X_ORIGINAL_URL? when available.

[peaceablewhale]

Targetting 2.8.1 due to #8974.

[Denis-de-Bernardy]

sadly, $is_IIS is defined further down in wp-settings.php so can't be used here.

[peaceablewhale]

Oops... thanks for the note. Use $_SERVERSERVER_SOFTWARE?,'Microsoft-IIS')!==false then...

[filosofo]

Why not just test whether IIS-specific PHP functions, such as iis_add_server(), exist?

[peaceablewhale]

Replying to filosofo:

Why not just test whether IIS-specific PHP functions, such as iis_add_server(), exist?

because those functions will not work when PHP is installed on IIS via FastCGI.

[Denis-de-Bernardy]

commit is for 5682.patch

[peaceablewhale]

Actually, both this and #10169 do not affect IIS 7.5. Should we exclude it from the patch? Mainly a performance concern.

[Denis-de-Bernardy]

I don't think so. We'd end up losing on the performance front for all other IIS users, and potentially others (version_compare() is very slow).

[peaceablewhale]

How about using (preg_match("/Microsoft-IIS\/(\d+(?:\.\d+)?)/",$_SERVERSERVER_SOFTWARE?,$iis_version)===1 && $iis_version[1]<=7)?

[Denis-de-Bernardy]

preg_match is going to slow things down

[ruslany]

I suggest resolve this bug as won't fix. When PHP is run on IIS 5.1 and IIS 6.0 with FastCGI (which is the recommended way to run PHP on IIS), then the REQUEST_URI will be set correctly. When PHP is run on IIS 7 with FastCGI and there is a URL Rewrite Module installed then the REQUEST_URI will be set correctly as well. So the only cases when it will not be set are: 1. When PHP is run as ISAPI - this is not recommended way of running PHP and it should be discouraged 2. When PHP is run on IIS 7 with FastCGI but there is no FastCGI patch applied. This will be extremely rare case as the main reason REQUEST_URI is used is to support pretty permalinks. Those require IIS 7 URL rewrite module installed. When that module is installed it also installs the FastCGI patch for REQUEST_URI.

[Denis-de-Bernardy]

As much as I agree with your arguments, the general trend here is that this should still get fixed as it'll affect some users.

[ruslany]

There is another problem with this patch. With this patch the WordPress will start using HTTP_X_ORIGINAL_URL server variable on IIS instead of the proper REQUEST_URI. Using that server variable is not recommended by IIS team as its value can be controlled by client by setting the HTTP header "x-original-url". This may be a potential security issue. Using of REQUEST_URI is recommended because it is set by IIS only and cannot be controlled by client.

IIS team has deliberately added REQUEST_URI support to all IIS versions so that workarounds like this one could be avoided.

[Denis-de-Bernardy]

ah, that is a much better point. needs another patch then. :-|

@peaceablewhale: what does the $_SERVER[SERVER_SOFTWARE] look like? a regexp sounds in order then, as in:

preg_match("/^{Microsoft-IIS\/(\d+(?:\.\d+)?)/", ...}

note the anchor at the beginning (^{). Without it, preg_match won't scale.}

[Denis-de-Bernardy]

preg_match("/^Microsoft-IIS\/(\d+(?:\.\d+)?)/", ...

even

[ruslany]

Extra condition to check for php_sapi_name()

[ruslany]

I think the patch 5682_fcgi.patch may work better. It still uses strpos instead of regex, which should be faster. Plus it checks that the PHP is not run via FastCGI, as with FastCGI the REQUEST_URI is set correctly on all versions of IIS.

[Denis-de-Bernardy]

Contrary to what the php docs say, anchored regex is *much* faster than strpos on a highly loaded server. So seriously advising an anchored regex if we can get one in there.

[Denis-de-Bernardy]

for the fasted test, you'd go:

empty(_$SERVER['REQUEST_URI']) || php_sapi_name() != 'cgi-fcgi' && preg_match(/^...)

[ruslany]

Updated patch as per ddebernardy suggestions

[ruslany]

The main reason regex was proposed in a first place was to extract the version information from the SERVER_SOFTWARE variable. As there is no need to do that, strpos seemed to do the job. However, if you say that anchored regex is faster that's fine by me.

I've updated the patch (5682_fcgi.2.patch) to use preg_match and also re-arranged the condition operands for faster test as per suggestion from ddebernardy.

[Denis-de-Bernardy]

I'm +1 for the latest patch

[peaceablewhale]

+1 to the latest patch too. However, is it better to add a () to the second part of the new statement?

empty( $_SERVER['REQUEST_URI'] ) || ( php_sapi_name() != 'cgi-fcgi' && preg_match( '/^Microsoft-IIS\//' , $_SERVER['SERVER_SOFTWARE'] ) > 0 )

[Denis-de-Bernardy]

for readability's sake only. it's not needed in php.

false

true && true = false

( true && true ) = true

[Denis-de-Bernardy]

doh, trac got rid of the or thingies.

[peaceablewhale]

Improving readability should be good :)

Perhaps we can leave this to the committer.

[azaozz]

(In [11661]) Improved $_SERVERREQUEST_URI? test for IIS, props ruslany, fixes #5682 for 2.8

[azaozz]

(In [11662]) Improved $_SERVERREQUEST_URI? test for IIS, props ruslany, fixes #5682 for trunk