r73381 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r73380‎ | r73381 | r73382 >
Date:15:38, 20 September 2010
Author:platonides
Status:resolved (Comments)
Tags:
Comment:
Follow up r72799, using MW_INSTALL_PATH and moving the logic to find maintenance/commandLine.inc to a common file.
Modified paths:
  • /trunk/extensions/Maps/test/MapsCoordinateParserTest.php (modified) (history)
  • /trunk/extensions/Maps/test/MapsDistanceParserTest.php (modified) (history)
  • /trunk/extensions/Maps/test/commandLine.inc (added) (history)

Diff [purge]

Index: trunk/extensions/Maps/test/MapsCoordinateParserTest.php
——@@ -1,14 +1,8 @@
22 <?php
33
44 require_once 'PHPUnit/Framework/TestCase.php';
 5+require_once dirname(__FILE__) . '/commandLine.inc';
56
6 -// Trick MW into thinking this is a command line script.
7 -// This is obviously not a good approach, as it will not work on other setups then my own.
8 -unset( $_SERVER['REQUEST_METHOD'] );
9 -$argv = array( 'over9000failz' );
10 -( include dirname(__FILE__) . '/../../../phase3/maintenance/commandLine.inc' ) or
11 -require_once '../../../smw/maintenance/commandLine.inc';
12 -
137 /**
148 * MapsCoordinateParser test case.
159 *
Index: trunk/extensions/Maps/test/MapsDistanceParserTest.php
——@@ -1,14 +1,8 @@
22 <?php
33
44 require_once 'PHPUnit/Framework/TestCase.php';
 5+require_once dirname(__FILE__) . '/commandLine.inc';
56
6 -// Trick MW into thinking this is a command line script.
7 -// This is obviously not a good approach, as it will not work on other setups then my own.
8 -unset( $_SERVER['REQUEST_METHOD'] );
9 -$argv = array( 'over9000failz' );
10 -( include dirname(__FILE__) . '/../../../phase3/maintenance/commandLine.inc' ) or
11 -require_once '../../../smw/maintenance/commandLine.inc';
12 -
137 /**
148 * MapsDistanceParser test case.
159 *
Index: trunk/extensions/Maps/test/commandLine.inc
——@@ -0,0 +1,23 @@
 2+<?php
 3+
 4+// Trick MW into thinking this is a command line script.
 5+// This is obviously not a good approach, as it will not work on other setups then my own.
 6+unset( $_SERVER['REQUEST_METHOD'] );
 7+$argv = array( 'over9000failz' );
 8+$optionsWithArgs = array();
 9+if ( count( $_REQUEST ) ) {
 10+ die( "This is a pseudo-command line script" );
 11+}
 12+
 13+if ( getenv( 'MW_INSTALL_PATH' ) !== false ) {
 14+ $IP = getenv( 'MW_INSTALL_PATH' );
 15+} else {
 16+ foreach( array( '../../../phase3', '../../../smw' ) as $rel ) {
 17+ if ( file_exists( dirname( __FILE__ ) . "/$rel/maintenance/commandLine.inc" ) ) {
 18+ $IP = dirname( __FILE__ ) . "/$rel";
 19+ break;
 20+ }
 21+ }
 22+}
 23+require( "$IP/maintenance/commandLine.inc" );
 24+
Property changes on: trunk/extensions/Maps/test/commandLine.inc
___________________________________________________________________
Added: svn:eol-style
125 + native

Follow-up revisions

RevisionCommit summaryAuthorDate
r73382Follow r73381. That dangling $IP could end up in a remote include vulnerability.platonides15:40, 20 September 2010
r73383Fix r73381.platonides15:41, 20 September 2010
r75378Folllow up r73381. Add ../.. path as mentioned in CR.platonides20:10, 25 October 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r72799Changing paths so that they work both on Unix and Windows....platonides12:20, 11 September 2010

Comments

#Comment by Catrope (talk | contribs)   18:47, 23 October 2010

... what?

The standard way is to either use MW_INSTALL_PATH assume the wiki root is in ../.. . That way you need zero assumptions about paths.

#Comment by Platonides (talk | contribs)   20:01, 25 October 2010

It turns out, jeroendedauw has it in ../../../smw the phase3 is for the checkout model where phase3 and extensions are siblings. I should have also included ../.., though.

#Comment by Catrope (talk | contribs)   19:35, 26 October 2010

I understand what Jeroen's checkout model looks like based on that code. The standard behavior, however, is to look for MW_INSTALL_PATH and assume the normal file layout (extensions as a subdir of phase3) if it's not set.

#Comment by Platonides (talk | contribs)   21:01, 26 October 2010

So you think that I should break it for him?

PS: I added the ../.. in r75378

#Comment by Catrope (talk | contribs)   21:16, 26 October 2010

With ../.. in there it'll be fine.

Status & tagging log