Bug 484855 - mediawiki api.php has bad path
mediawiki api.php has bad path
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: mediawiki (Show other bugs)
10
x86_64 Linux
medium Severity medium
: ---
: ---
Assigned To: Axel Thimm
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-10 06:08 EST by Joel Uckelman
Modified: 2009-07-31 13:37 EDT (History)
7 users (show)

See Also:
Fixed In Version: 1.15.1-48.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-23 11:15:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
proposed (886 bytes, patch)
2009-03-10 17:00 EDT, Bret McMillan
no flags Details | Diff
FIrst pass at potential changes to mediawiki-1.14.0-commoncode.patch (3.76 KB, patch)
2009-03-12 15:36 EDT, Bret McMillan
no flags Details | Diff

  None (edit)
Description Joel Uckelman 2009-02-10 06:08:50 EST
Description of problem:

The MediaWiki API is accessible from api.php, which Fedora ships as /usr/share/mediawiki/api.php. By default, there is no symlink to api.php in /var/www/wiki, presumably so that anyone who wants to use api.php has to expose it purposely. That's fine, except that adding a symlink in /var/www/wiki isn't sufficient to get api.php to work.

The line

  require (dirname(__FILE__) . '/includes/WebStart.php');

in api.php fails because dirname(__FILE__) is incorrect when running api.php from /var/www/wiki/api.php.

index.php is symlinked from /usr/share/mediawiki/, and works out of the box. This is due to having the following lines, instead:

  $DIR=getcwd();
  chdir('/usr/share/mediawiki');

  // Initialise common code
  $preIP = dirname( __FILE__ );

  require ($preIP . '/includes/WebStart.php');

If these are substituted into api.php, then it will work properly from a symlink, as dirname(__FILE__) has the correct value after the call to chdir().

api.php should be modified so that it works in the expected way.


Version-Release number of selected component (if applicable):

mediawiki-1.13.3-42.fc10.x86_64



How reproducible:

Always.

Steps to Reproduce:
1. cd /var/www/wiki
2. ln -s /usr/share/mediawiki/api.php
3. Browse to api.php.
  
Actual results:

[Tue Feb 10 03:56:21 2009] [error] [client 145.18.21.54] PHP Fatal error:  requi
re_once() [<a href='function.require'>function.require</a>]: Failed opening requ
ired '/var/www/wiki/StartProfiler.php' (include_path='.:/usr/share/pear:/usr/sha
re/php') in /usr/share/mediawiki/includes/WebStart.php on line 78

Expected results:

api.php should run without error.
Comment 1 Axel Thimm 2009-02-28 13:31:37 EST
(In reply to comment #0)
> The MediaWiki API is accessible from api.php, which Fedora ships as
> /usr/share/mediawiki/api.php. By default, there is no symlink to api.php in
> /var/www/wiki, presumably so that anyone who wants to use api.php has to expose
> it purposely. That's fine, except that adding a symlink in /var/www/wiki isn't
> sufficient to get api.php to work.

No, that's not the intention. Instead use the $IP method to get the php source code under /usr/share/mediawiki, just like mediawiki instances themselves access these files.
Comment 2 Joel Uckelman 2009-02-28 14:00:54 EST
(In reply to comment #1)
> 
> No, that's not the intention. Instead use the $IP method to get the php source
> code under /usr/share/mediawiki, just like mediawiki instances themselves
> access these files.

I can't make out from your reply what the correct way is to do this. From where am I supposed to use $IP?
Comment 3 Axel Thimm 2009-02-28 16:35:41 EST
http://www.mediawiki.org/wiki/Manual:$IP
Comment 4 Joel Uckelman 2009-03-01 08:14:15 EST
I know what the $IP variable is for---it holds the path to the MW installation. But I don't see how one can use it when making a call to api.php from something which doesn't have access to LocalSettings.php.
Comment 5 Axel Thimm 2009-03-01 08:48:44 EST
LocalSettings.php is a the top of every mediawiki instance.
Comment 6 Joel Uckelman 2009-03-01 09:07:44 EST
We're clearly not talking about the same thing here. I'm not saying that I don't know where LocalSettings.php is or that I can't find it. I'm saying that I see no way to get the value of $IP from a process which doesn't have permission to read LocalSettings.php or maybe isn't even on the same machine as LocalSettings.php.

If I install MW from source (rather than from the RPM) and I can get to the wiki from http://www.example.com/wiki, then I can access api.php from http://www.example.com/wiki/api.php (unless $wgEnableAPI is set to false). In particular, this means that I can call api.php from scripts which don't have read access for LocalSettings.php, or aren't even on the same machine as the wiki is.

In contrast, this does not work the RPM, unless you modify api.php and add a symbolic link (as in my original message). This is a deviation from the default behavior, so either it should be documented someplace or the default behavior should be restored.
Comment 7 Axel Thimm 2009-03-01 10:51:21 EST
api.php calls WebStart.php which read in LocalSettings.php. It die()s if LocalSettings.php does not exist.

You even mention $wgEnableAPI, which is read from LocalSettings.php. If you argue that one can use api.php w/o access to LocalSettings.php, but $wgEnableAPI which controls whether you are allowed to use the api actually lives in LocalSettings.php, then this contradicts the very existence of $wgEnableAPI.
Comment 8 Joel Uckelman 2009-03-01 11:02:17 EST
(In reply to comment #7)
> api.php calls WebStart.php which read in LocalSettings.php. It die()s if
> LocalSettings.php does not exist.
> 
> You even mention $wgEnableAPI, which is read from LocalSettings.php. If you
> argue that one can use api.php w/o access to LocalSettings.php, but
> $wgEnableAPI which controls whether you are allowed to use the api actually
> lives in LocalSettings.php, then this contradicts the very existence of
> $wgEnableAPI.

I am *not* saying that api.php can't read LocalSettings.php---clearly api.php would not work at all if it couldn't. The process running api.php and the process calling it via an HTTP POST are not the same process. I am talking about the latter, not the former.

Please look again at the scenario I described in Comment #6. I am passing parameters to api.php via an HTTP POST. I am not invoking api.php myself, Apache is invoking it in response to the POST. Whether the process which is POSTing can read LocalSettings.php is irrelevant for whether the POST will succeed.
Comment 9 Axel Thimm 2009-03-01 12:54:54 EST
Sorry, I now understand what you mean - I thought you were trying to write an extension.

Reopening and indeed the proposed solution at the very top of your comments is probably the best way to fix this.
Comment 10 Joel Uckelman 2009-03-01 15:05:13 EST
Ok, glad we're together now. It hadn't occurred to me that anyone would be trying to call api.php directly, so I didn't think to say that I was doing it via an HTTP POST. Thanks for your persistence.
Comment 11 Bret McMillan 2009-03-10 16:48:59 EDT
What I'm seeing in 1.14 is that api.php differs from upstream in a pretty wrong way...

diff -up ./WebStart.php.orig ./WebStart.php
--- ./WebStart.php.orig	2009-03-10 16:16:57.000000000 -0400
+++ ./WebStart.php	2009-03-10 16:17:10.000000000 -0400
@@ -103,14 +103,14 @@ if ( defined( 'MW_CONFIG_CALLBACK' ) ) {
 	# LocalSettings.php is the per site customization file. If it does not exit
 	# the wiki installer need to be launched or the generated file moved from
 	# ./config/ to ./
-	if( !file_exists( "$DIR/LocalSettings.php" ) ) {
+	if( !file_exists( "$IP/LocalSettings.php" ) ) {
 		require_once( "$IP/includes/DefaultSettings.php" ); # used for printing the version
 		require_once( "$IP/includes/templates/NoLocalSettings.php" );
 		die();
 	}
 
 	# Include site settings. $IP may be changed (hopefully before the AutoLoader is invoked)
-	require_once( "$DIR/LocalSettings.php" );
+	require_once( "$IP/LocalSettings.php" );
 }
 wfProfileOut( 'WebStart.php-conf' );


See http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/WebStart.php?view=annotate

DIR never gets set in api.php, causing the following log messages to show whenever you try to access it:

[Tue Mar 10 16:04:43 2009] [error] [client 127.0.0.1] PHP Notice:  Undefined variable: DIR in /usr/share/mediawiki/includes/WebStart.php on line 106, referer: http://localhost/wiki/index.php/Main_Page


I found this when trying to debug why the search suggestions weren't working on my instance.
Comment 12 Bret McMillan 2009-03-10 16:50:46 EDT
Raising severity and priority a bit since this breaks api.php in general.
Comment 13 Bret McMillan 2009-03-10 17:00:55 EDT
Created attachment 334714 [details]
proposed 

Here's a potential patch, but it's probably worth comparing against trunk and seeing why code committed back in October still isn't into the source tarball we're using yet...
Comment 14 Bret McMillan 2009-03-12 15:36:22 EDT
Created attachment 335006 [details]
FIrst pass at potential changes to mediawiki-1.14.0-commoncode.patch

Ok, I've eyeballed this patch, and tried to weed out the rogue DIR references.

I'll be building local rpms and testing...
Comment 15 Jon Stanley 2009-05-25 08:51:56 EDT
Bret -

Did the patch help? Unfortunately it's been brought to my attention that this is breaking api.php on the fedoraproject.org wiki :(
Comment 16 Ricky Zhou 2009-05-25 13:11:50 EDT
Does anybody know why mediawiki-1.14.0-commoncode.patch was necessary in the first place?  

I just did a test install using an unpatched upstream tarball where all of the files were symlinked into place, and it worked perfectly.  Based on this experience, the ideal situation for me would be one where /usr/share/mediawiki just contains roughly the contents of the mediawiki tarball without any patching.

So what do you say to just getting rid of the patch?  I haven't done specific testing of this yet, but I don't think taking it out would even break existing installs.
Comment 17 Ricky Zhou 2009-06-05 00:05:35 EDT
Ping?  This bug has been open for almost 4 months now, and it's totally breaking mediawiki API usage, as well as the file:

 * img_auth.php
 * opensearch_desc.php
 * profileinfo.php
 * redirect.php
 * thumb.php
 * trackback.php

and anything I missed that uses includes/WebStart.php without the $DIR hack.
Comment 19 Axel Thimm 2009-06-13 14:20:24 EDT
(In reply to comment #16)
> So what do you say to just getting rid of the patch?  I haven't done specific
> testing of this yet, but I don't think taking it out would even break existing
> installs.  

Did you check that farming works if you remove the patch? I'm quite sure it doesn't.
Comment 20 Ricky Zhou 2009-06-13 14:27:45 EDT
Sorry, I'm not sure what you mean by farming, just having multiple mediawiki instances on the same machine with separate data?
Comment 21 Ricky Zhou 2009-06-13 14:38:20 EDT
If that is what you mean, I just setup a second test instance with the stock upstream mediawiki-1.14.0.tar.gz.  You can see them at:

http://publictest1.fedoraproject.org/wiki/Main_Page
http://publictest1.fedoraproject.org/wiki2/Main_Page

(these may go down soon to avoid being spammed)
Comment 22 Ricky Zhou 2009-06-13 14:58:20 EDT
Here are two instances with unpatched mediawiki-1.15.0.tar.gz:

http://publictest1.fedoraproject.org/wiki3/Main_Page
http://publictest1.fedoraproject.org/wiki4/Main_Page

The procedure I used for setting them up was:

untar the tarball in /srv/web
mkdir /srv/web/w3
cd /srv/web/w3
ln -s ../../mediawiki-1.15.0/* .
rm config images
mkdir images
cp -r ../../mediawiki-1.15.0/config .
chown -r apache:apache config images

go to http://publictest1.fedoraproject.org/w3/ and setup the wiki

cp config/LocalSettings.php .
rm -rf config

add:

$wgArticlePath = "/wiki3/$1";
$wgUsePathInfo = true;

to LocalSettings.php

add:

Alias /wiki3 /srv/web/w3/index.php

<Directory /srv/web/w3>
  Options FollowSymLinks
</Directory>

to /etc/httpd/conf.d/mediawiki.conf

/etc/init.d/httpd restart

This is basically following upstream's documentation for setting up multiple instances (http://meta.wikimedia.org/wiki/MediaWiki_FAQ#Is_it_possible_to_install_more_than_one_wiki_.28MediaWiki.29_on_a_server.3F_How.3F)
Comment 23 Ricky Zhou 2009-06-15 14:29:46 EDT
For what it's worth, here are two test instances I just setup with a patched package:

http://publictest3.fedoraproject.org/wiki1/Main_Page
http://publictest3.fedoraproject.org/wiki2/Main_Page

I've set them read only, but if anybody sees any other issues/bugs with them, let me know.
Comment 24 Ricky Zhou 2009-06-15 16:37:20 EDT
Also, the patched package without the commoncode patch is now running the wikis at http://fedoraproject.org/wiki/ and http://smolts.org/smolt-wiki/ on the same servers.

We've verified that https://fedoraproject.org/w/api.php works as expected.
Comment 25 Ricky Zhou 2009-06-21 02:11:22 EDT
Ping?  Is there anything else stopping us from getting rid of the patch?
Comment 26 Ricky Zhou 2009-06-21 20:02:49 EDT
*** Bug 494362 has been marked as a duplicate of this bug. ***
Comment 27 Ricky Zhou 2009-06-21 20:03:47 EDT
*** Bug 494880 has been marked as a duplicate of this bug. ***
Comment 28 Ricky Zhou 2009-06-21 20:09:01 EDT
Two updates to this, I just marked bugs #494362 and #494880 as duplicates of this one because the reporters found some issues caused by the commoncode patch. The two issues are:

* The updateSearchIndex.php maintenance script is broken due to the undefined $DIR variable that the commoncode patch added.
* If $wgUploadDirectory is not defined, the image upload directory will default to /usr/share/mediawiki/images due to the use of $IP there. I will test if removing the patch and installing using the upstream-recommended symlinking method corrects this issue later today.
Comment 29 Ricky Zhou 2009-06-22 01:35:02 EDT
I just tested the second issue, and it does work fine in an unpatched mediawiki.

Axel, I know you must be pretty busy, so I just wanted to let you know that I'd be happy to comaintain mediawiki with you if needed.  I went through most of the mediawiki bugs in bugzlila and got most of them updated today - I hope that's helpful to you.
Comment 30 Ricky Zhou 2009-06-27 00:24:21 EDT
Ping?  It has been another two weeks since the last response on this bug.
Comment 31 Ricky Zhou 2009-07-06 23:55:02 EDT
Hi, it has been three weeks without any update now.  If you don't have time to look at this, I'd still be happy to comaintain this package with you.
Comment 32 Axel Thimm 2009-07-11 12:55:58 EDT
(In reply to comment #22)
> The procedure I used for setting them up was:
> ln -s ../../mediawiki-1.15.0/* .

The commoncode idea is two fold:

a) expose as few code to the outside as possible. The code under datadir is never directly accessible from the outside, so any security issue there will never surface in the commoncode setup. There had been some CVS even bugzilla'd here that didn't affect our package due to this.

b) ease the upgrade process: each major mediawiki has had a change in the directly structure resulting in the current discussion context of using symlinks instead of commoncode in broken or missing symlinks. So removing the patch seemingly results in working setup, but is less maintainable in the long term and would not survive package upgrades with changed symlink layout.

My basic concerns are safety and package upgrades, maybe the latter even more that the former. Absolutely safe package upgrades could only be done by dropping both commoncode and symlinking and using a copy-per-instance method. Which would result in people not knowing that they don't really upgrade mediawiki.

From the other two methods I prefer commoncode as it is safer both in upgrades and security views. Of course it has some undeniable bugs, but instead of throwing the baby with the water I think it is better to fix the patch and prepare it for upstream inclusion (again).
Comment 33 Fedora Update System 2009-07-11 16:55:06 EDT
mediawiki-1.15.0-47.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/mediawiki-1.15.0-47.fc10
Comment 34 Fedora Update System 2009-07-11 16:55:21 EDT
mediawiki-1.15.0-47.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/mediawiki-1.15.0-47.fc11
Comment 35 Ricky Zhou 2009-07-11 23:04:28 EDT
> a) expose as few code to the outside as possible. The code under datadir is
> never directly accessible from the outside, so any security issue there will
> never surface in the commoncode setup. There had been some CVS even bugzilla'd
> here that didn't affect our package due to this.
I would prefer that this be prevented via Apache configuration instead of patching code.

Here is an instance that I setup with mediawiki outside of the DocumentRoot using Aliases: http://publictest1.fedoraproject.org/wiki2/Main_Page

The Apache configuration I used is:

# Wiki 2
Alias /wiki2 /srv/web/w2/index.php

Alias /w2/index.php /srv/web/w2/api.php
Alias /w2/api.php /srv/web/w2/api.php
Alias /w2/opensearch_desc.php /srv/web/w2/opensearch_desc.php

# Remove this after installing.
Alias /w2/config /srv/web/w2/config

<Directory /srv/web/w2>
  Options FollowSymLinks
</Directory>

> b) ease the upgrade process: each major mediawiki has had a change in the
> directly structure resulting in the current discussion context of using
> symlinks instead of commoncode in broken or missing symlinks. So removing the
> patch seemingly results in working setup, but is less maintainable in the long
> term and would not survive package upgrades with changed symlink layout.
Each major upgrade also comes with database migrations as well (as well as the possibility of broken extensions and configuration file format changes).  This includes the update that you just pushed.  In either of these cases, serious breakage can happen if a user blindly updates.  This is why I think this is a situation where documenting the process and letting the user decide how to approach it is best.  

The specific set of symlinks that we are currently using for the Fedora wiki is:

api.php -> /usr/share/mediawiki/api.php
extensions -> /usr/share/mediawiki/extensions
includes -> /usr/share/mediawiki/includes
index.php -> /usr/share/mediawiki/index.php
languages -> /usr/share/mediawiki/languages
maintenance -> /usr/share/mediawiki/maintenance
opensearch_desc.php -> /usr/share/mediawiki/opensearch_desc.php
skins -> /usr/share/mediawiki/skins
StartProfiler.php -> /usr/share/mediawiki/StartProfiler.php

I don't think these are very likely to change soon (I can even ask upstream for their opinion on this, if you'd like).  In fact (and correct me if I'm wrong), the default installation from the package doesn't even expose api.php or opensearch_desc.php, which are very useful in a lot of wiki setups. 

Another alternative that I'd be totally happy with is if you could get these changes upstream, where they would receive a lot more testing and attention, and breakages would be caught and fixed quickly.  

> From the other two methods I prefer commoncode as it is safer both in upgrades
> and security views. Of course it has some undeniable bugs, but instead of
> throwing the baby with the water I think it is better to fix the patch and
> prepare it for upstream inclusion (again).  
So from my comments, above, I believe the security issue can be completely solved using the method I described.  I've also brought up two points about package upgrades:

a) There is absolutely no substitute for proper documentation and special user attention when it comes to major version upgrades.  When a person upgrades their mediawiki package, they need to do database upgrades, check extension compatibility and configuration option changes, and check for broken symlinks.  This checklist of things to do could be put in an UPGRADE.fedora or some other form of documentation (maybe even upstream).
b) Files/directories names such as:

api.php
extensions
includes
index.php
languages
maintenance
opensearch_desc.php
skins
StartProfiler.php

are very unlikely to change upstream, and even they do change, or new required files are added, users need to pay the same amount of attention to these details as db upgrades/extensions/config changes during an major upgrade.
Comment 36 Axel Thimm 2009-07-12 15:01:25 EDT
(In reply to comment #35)
> > a) expose as few code to the outside as possible. [...]
> I would prefer that this be prevented via Apache configuration instead of
> patching code.

In fact you are using just the same mechanism, just not under /usr/share but /srv/web assuming that Documentroot is not pointing there as well.

But actually your setup looks quite more complicated and error prone, see for example:

> Alias /w2/index.php /srv/web/w2/api.php

> > b) ease the upgrade process: [...]
> Each major upgrade also comes with database migrations as well [...]

Sure, one can try to keep the pain low or even tell users to do a manual upgrade to catch all issues. In the latter case one could even consider not to ship any /var/www example, or maybe not to package this at all. I disagree with a radical approach and I prefer offering the common code in a package setup and let the user just do the database upgrades themselves (when needed).

> So from my comments, above, I believe the security issue can be completely
> solved using the method I described.

I don't agree, the setup looks error prone (the config you wrote already contains an error) and you are symlinking maintenance into the web accessible area which does contain quite powerful scripts.

Please check the current package. It does expose api.php and hopefully w/o any bugs. We can discuss exposing further parts that are really needed on the URL line. But a blind whitelisting of all contributed code under $ROOT isn't wise, also maintenance is for running locally, and for example StartProfiler.php is for special needs requiring manual editing by upstream already.
Comment 37 Ricky Zhou 2009-07-12 16:18:54 EDT
(In reply to comment #36)
> > Alias /w2/index.php /srv/web/w2/api.php
Sorry for the typo, but apart from this, the method I described solves the security concerns just fine, and you will have a hard time making a case that this is more error-prone than patching the code, as we have already seen in the multiple breakages in the various bugs related to the patch.

Here is a fixed configuration that fixes the typo and adds two missing aliases.  Note once again that /srv/web/w2 is *outside* the DocumentRoot.

# Wiki 2
Alias /wiki2 /srv/web/w2/index.php

Alias /w2/index.php /srv/web/w2/index.php
Alias /w2/api.php /srv/web/w2/api.php
Alias /w2/opensearch_desc.php /srv/web/w2/opensearch_desc.php
Alias /w2/skins /srv/web/w2/skins
Alias /w2/images /srv/web/w2/images

# Remove this after installing.
Alias /w2/config /srv/web/w2/config

<Directory /srv/web/w2>
  Options FollowSymLinks
</Directory>

> Sure, one can try to keep the pain low or even tell users to do a manual
> upgrade to catch all issues. In the latter case one could even consider not to
> ship any /var/www example, or maybe not to package this at all. I disagree with
> a radical approach and I prefer offering the common code in a package setup and
> let the user just do the database upgrades themselves (when needed).
Yes, a user must do database upgrades themselves, and similarly, they must fix broken symlinks themselves on an install.  Patching mediawiki does cannot take these steps away from a package upgrade, which is why I question the gain of the patch.

> I don't agree, the setup looks error prone (the config you wrote already
> contains an error) and you are symlinking maintenance into the web accessible
> area which does contain quite powerful scripts.
Again, if you want to talk about error prone methods, look at this bug, bug 494362, bug 494880, and the list of files I provided in comment 17, all of which are still broken due to the patch.  The config I gave contained some simple typos that I have fixed above.  I don't think it is fair to pick on that first attempt in calling the entire method error-prone.  If I wasn't clear about this, I symlinked the maintenance scripts into a path outside of the DocumentRoot.  These are completely unexposed via apache using the configuration that I posted.

> Please check the current package. It does expose api.php and hopefully w/o any
> bugs. We can discuss exposing further parts that are really needed on the URL
> line. But a blind whitelisting of all contributed code under $ROOT isn't wise,
> also maintenance is for running locally, and for example StartProfiler.php is
> for special needs requiring manual editing by upstream already.  
I think I have already fixed any security-related concerns with the symlinking method.  With the configuration that I described, there are exactly five paths under mediawiki exposed over the web:

index.php
api.php
opensearch_desc.php
skins
images

Once again, any other paths are outside of DocumentRoot and thus not accessible over the web.  

Not that the api.php issue is central to my argument about the patch, but if I:

yum install mediawiki
cp -r /var/www/wiki /var/www/html
/etc/init.d/httpd restart

and run the setup scripts, I get a setup like this:
http://publictest2.fedoraproject.org/wiki/

The CSS and images are missing from the installation to the final result, and there is no api.php or opensearch_desc.php.  My apologies if I didn't set this up properly, but I did not find any documentation for how to set things up to account for the Fedora-only patches.  

Hopefully, in this comment, I have put the security concerns to rest.  The method is not more error-prone than the current method, and it also happens to be the solution suggested upstream at http://meta.wikimedia.org/wiki/MediaWiki_FAQ#Is_it_possible_to_install_more_than_one_wiki_.28MediaWiki.29_on_a_server.3F_How.3F.  This brings the benefit of being widely tested by upstream, as none of their code is modified in any way.  Once again, only a select set of paths are exposed to the web.

Additionally, as I mentioned above, major mediawiki package updates already require significant user attention in the form of database upgrades, extension compatibility, and possible config option changes.  Thus, I don't think the patch gains anything in the way of making upgrades simpler.  Instead, this could be addressed by including an upgrade checklist in the documentation which would mention the the above issues, as well as the issues of checking that files are symlinked properly in each wiki instance.
Comment 38 Aryeh Gregor 2009-07-12 17:51:14 EDT
Hi, I'm a MediaWiki developer, and Ricky pointed this discussion out to me.  Your patch looks like it's likely to break a lot of things (and in fact, it looks like it already has).  It's because of issues like this that we're forced to recommend that our users avoid all packaged versions of MediaWiki.  Usually our first piece of advice when a user asks for support of packaged MediaWiki is to download the official tarball.  I've seen that fix the problem more than a few times.  (Debian has problems with this kind of thing too.)

This is unfortunate, of course, since package managers are so convenient.  I also understand that it's reasonable to put everything in /usr/share if you have root access -- we don't do that by default because we need to support shared hosts.  Why don't you post the patch upstream at https://bugzilla.wikimedia.org/?  I'd be happy to review it and commit it for 1.16 once all the problems are hashed out, and you can backport it to Fedora's 1.15.
Comment 39 Axel Thimm 2009-07-14 03:05:58 EDT
(In reply to comment #38)
> I also understand that it's reasonable to put everything in /usr/share if you
> have root access -- we don't do that by default because we need to support
> shared hosts.

The patch is not forcing anything as root, in fact all Fedora standard web apps are running as the apache user. The patch just separated code from data, something the code has been evolving to anyway. Any user can still setup as many shared hosts he likes.

> I'm a MediaWiki developer

Which is great since I hope you could polish the code/data separating some more and push it into the code.
Comment 40 Aryeh Gregor 2009-07-14 15:13:17 EDT
(In reply to comment #39)
> The patch is not forcing anything as root, in fact all Fedora standard web apps
> are running as the apache user.

Users on shared hosting cannot write to /usr/share or install packages.  They have only FTP access to a subdirectory of the web root, usually not even shell access.  For these users, we need to allow MediaWiki to run when all the files are in the web root, since that's the only place they can upload files.  We *cannot* require this kind of file layout upstream.

However, we could support installation into /usr/share as an alternative, for users who want to do that (e.g., installing from packages).  Since "all in web root" installation works fine for users with root access too, that will certainly remain the default in the tarball.

> Which is great since I hope you could polish the code/data separating some more
> and push it into the code.  

To do this correctly would require a considerable amount of work.  Your current patch is unsalvageable.  You're trying to manually hack every single reference to a path, which is unmaintainable.  You can probably achieve something like what you're after (minimal code in /var/www) without any code hacks, something like this:

1) Put the entire source tarball in /usr/share/mediawiki/.

2) Add a file named index.php to /var/www/wiki (or whatever), containing:

<?php
define( 'MW_INSTALL_PATH', '/usr/share/mediawiki' );
chdir( MW_INSTALL_PATH );
require_once( MW_INSTALL_PATH . '/index.php' );

3) Create similar files in /var/www/wiki named api.php, img_auth.php, opensearch_desc.php, redirect.php, thumb.php, and trackback.php.  These appear to be all our entry points post-installation.  They should be identical but with the name of the file changed on the last (fourth) line.

4) ln -s /usr/share/mediawiki/skins /var/www/wiki/skins.  This includes static files that are served directly to users and so must be in the web root.  The current spec file doesn't copy the required skins/ files, so styles and scripts all break.  From skins/, *.css and *.js files need to be in the right place in /var/www/wiki/skins, while everything else needs to be in the right place in /usr/share/mediawiki/skins, so if you wanted you could split it up.

5) Copy or move config/ to /var/www/wiki, and change this one line in config/index.php:

-$IP = dirname( dirname( __FILE__ ) );
+$IP = '/usr/share/mediawiki';

6) ln -s /usr/share/mediawiki/extensions /var/www/wiki/extensions.  This will make the extensions' PHP files' web-readable, but it's needed to get CSS/JS/images to work, unless you want to try splitting up the files for every extension ever written.

7) Copy images/, and make sure it's writable by the web server.

That should mostly work, probably.  It will certainly work better than what you have now, which is completely broken -- stylesheets aren't even loaded because you didn't put them in the web root.  And you missed a number of entry points (index.php and api.php aren't the only ones).

It would, of course, be simpler to just symlink everything to /usr/share/mediawiki and be done with it.  You could configure the web server to ignore any PHP files except for the entry points, if you're worried about users directly executing backend files.  That has the advantage of actually working, and being pretty easy, with no maintenance burdens.

I'm told the Debian package works.  Perhaps you could take a look at that.  If you or anyone else is willing to write a script that will generate a minimal web root as described here, or in any other sane fashion, I'd be happy to review it and commit it upstream so you don't have to maintain it.
Comment 41 Fedora Update System 2009-07-19 06:20:17 EDT
mediawiki-1.15.1-48.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 42 Fedora Update System 2009-07-19 06:33:36 EDT
mediawiki-1.15.1-48.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 43 Ricky Zhou 2009-07-19 09:38:31 EDT
Reopening this bug, since the patch being discussed is still in the package (I'd be happy to move discussion into another bug if that would be preferred though).
Comment 44 Ricky Zhou 2009-07-23 00:17:42 EDT
Ping?  There hasn't been any maintainer response on this in over a week again.
Comment 45 Axel Thimm 2009-07-23 01:16:12 EDT
(In reply to comment #44)
> Ping?  There hasn't been any maintainer response on this in over a week again.  

There was a new package pushed out 4 days ago,fixing the api.php issue, the push was registered into this bug and you consider the maintainer absent?
Comment 46 Ricky Zhou 2009-07-23 08:12:37 EDT
Sorry if I am being unreasonable here, but I would like to see a response to the comments that upstream was kind enough to make about how mediawiki is currently packaged.
Comment 47 Axel Thimm 2009-07-23 11:15:41 EDT
(In reply to comment #40)
> (In reply to comment #39)
> > The patch is not forcing anything as root, in fact all Fedora standard web apps
> > are running as the apache user.
> 
> Users on shared hosting cannot write to /usr/share or install packages.  They
> have only FTP access to a subdirectory of the web root, usually not even shell
> access.  For these users, we need to allow MediaWiki to run when all the files
> are in the web root, since that's the only place they can upload files.  We
> *cannot* require this kind of file layout upstream.

(In reply to comment #46)
> Sorry if I am being unreasonable here, but I would like to see a response to
> the comments that upstream was kind enough to make about how mediawiki is
> currently packaged.  

There is an argument about mediawiki not belonging under /usr/share, because users have no write access there. Both methods we were discussing used /usr/share, and even if not, even if mediawiki would simply dump its contents under /var, /tmp or elsewhere it would still not be allowed to be user editable in a packaged version of mediawiki.

So I see simetrical's (valid) arguments as mostly against packaged content in general. But a user can still cp -a /var/www/wiki /home/user/public_html/superwiki and benefit from the packaged version. This also doesn't distinguish between the two methods we were discussing, although I think the commoncode approach makes it easier for users to upgrade (they still have to cater for their database wherever it lives, but that's a problem every packaged webapp has).
Comment 48 Ricky Zhou 2009-07-23 20:49:44 EDT
I don't see us making much progress with the discussion on this bug, so at the suggestion of a few members, I have filed a FESCo ticket about this at https://fedorahosted.org/fesco/ticket/225 (you are CCed on it).

Note You need to log in before you can comment on or make changes to this bug.