r49833 MediaWiki - Code Review archive

This looks like it'll fail wildly if SCRIPT_NAME is set to the PHP interpreter, as it's apparently supposed to for CGI mode. It also appears to introduce a new server variable called 'URL' which I'm not sure exists consistently.

How widely have you tested this?

For the record, this has been tested to work on IIS/CGI (patch author's setup), Apache/CGI on Windows and Apache/module on Linux (my setups). We didn't test any setups involving FastCGI (which is the setup the bug was reported on by Bryan in the first place), but that should probably be done.

Just tested it as working. See bugzilla comments.

Just because it works for you doesn't mean it works for everyone. There are a lot of possible setups out there. It seems like a much better idea to use the same settings as were always used, by default, and only fall back to these other ones if those are detected as being incorrect or absent.

Could do something like this:

if( isset( $_SERVER['SCRIPT_URL'] ) ) {
	$url1 = $_SERVER['SCRIPT_URL'];
} else {
	$url1 = $_SERVER['PHP_SELF'];
}
if( isset( $_SERVER['SCRIPT_NAME'] ) ) {
	$url2 = $_SERVER['SCRIPT_NAME'];
} else {
	$url2 = $_SERVER['URL'];
}
$urlCmp = "$wgScriptPath/api$wgScriptExtension";

if( strcmp( $urlCmp, $url1 ) != 0 || strcmp( $urlCmp, $url2 ) != 0 ) {
	wfHttpError( 403, 'Forbidden',
		'API must be accessed through the primary script entry point.' );
	return;
}

Is there a reason to use strcmp instead of != ?

With mod-rewrite I'm getting the forbidden error. I'm rewriting with apache from https://server/wiki/ to http://server/mediawiki/. I have set $wgScriptPath to "/wiki". in the problematic code the variable "$url" is set to "/wiki/api.php" and the string comparison fails. As I do not really understand what the comparison is for, I temporarily hardcoded /mediawiki/ instead of $wgScriptPath/ there and it works. It could be a solution to replace the $wgScriptPath variable with the php-script's path

If the 'real' path to your PHP files is /mediawiki , that's what your $wgScriptPath should be. And if your $wgScriptPath is misconfigured, it's not very surprising that this check fails.

It's configured as described at http://www.mediawiki.org/wiki/Manual:Short_URL/Apache_Rewrite_rules . Everything is working properly for some years now except access to the api.php

Then that guide is wrong. $wgScriptPath should be the *real* path to index.php , not the fake path that only works because Apache rewrites it.

Setting the variable to the real path makes the complete wiki unuseable because it's value is a used as a prefix to the path - the real one is not accessible via http.

Wrong. $wgScriptPath is the URL path. For the filename path use $IP

I have found a solution for that issue as well as a solution for the same issue I had on includes/RawPage.php:

For sites, which are using Apache's "rewrite", it is neccessary to turn set "RewriteEngine On" for the complete VirtualHost; otherwise (when turned on only per directory) $_SERVER['SCRIPT_URL'] will be empty and $_SERVER['PHP_SELF'] will be used to determine $url.

Manual:Short_URL/Apache_Rewrite_rules is correct, $wgScriptPath must be set to the public visible url.

$_SERVER['URL'] clearly does not exist consistently, since it is IIS-specific. It is defined in the IIS documentation (http://msdn.microsoft.com/en-us/library/ms524602.aspx) but not in the CGI specification (http://hoohoo.ncsa.illinois.edu/cgi/env.html). SCRIPT_NAME supposedly exists in IIS as well, so I wonder if the else case was reached in any of the test setups.

According to comments in PHP's sapi/cgi/cgi_main.c, Apache does indeed screw up SCRIPT_NAME, but PHP has had code to correct for that since the dark ages (PHP 4.3.0), to restore the well-known and documented behaviour, which is that SCRIPT_NAME is the URL of the script and can be used for self-referring URLs. If the fixup code is activated, the original script name is put in ORIG_SCRIPT_NAME.

However, if the php.ini setting cgi.fix_pathinfo is off, then the PHP 4.2 behaviour is recovered and SCRIPT_NAME will be broken and unusable. But we can emulate the disabled behaviour, which is simply to use the apache-specific variable REDIRECT_URL instead, if it is available.

Note that this revision does appear to disable the security check entirely. SCRIPT_NAME does not include PATH_INFO. I propose we just remove the whole thing. It's only useful for IE 3 and earlier, IE 4 will respect the Content-Type header as long as it's not in the well-known danger list.