r110122 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r110121‎ | r110122 | r110123 >
Date:14:15, 27 January 2012
Author:reedy
Status:resolved (Comments)
Tags:filebackend 
Comment:
Fix undefined $dirRoot
Modified paths:
  • /trunk/phase3/includes/filerepo/backend/FSFileBackend.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/backend/FSFileBackend.php
——@@ -1,4 +1,4 @@
2 -<?php
 2+g<?php
33 /**
44 * @file
55 * @ingroup FileBackend
——@@ -366,7 +366,7 @@
367367 if ( !empty( $params['noAccess'] ) ) {
368368 if ( !file_exists( "{$contRoot}/.htaccess" ) ) {
369369 wfSuppressWarnings();
370 - $ok = file_put_contents( "{$dirRoot}/.htaccess", "Deny from all\n" );
 370+ $ok = file_put_contents( "{$contRoot}/.htaccess", "Deny from all\n" );
371371 wfRestoreWarnings();
372372 if ( !$ok ) {
373373 $storeDir = "mwstore://{$this->name}/{$shortCont}";

Follow-up revisions

RevisionCommit summaryAuthorDate
r110127Fix spurious character from r110122reedy15:03, 27 January 2012

Comments

#Comment by MarkAHershberger (talk | contribs)   14:41, 27 January 2012

Are you sure this shouldn't be $dir?

#Comment by Reedy (talk | contribs)   15:03, 27 January 2012

Somewhat 

 		if ( !empty( $params['noAccess'] ) ) {
 			if ( !file_exists( "{$contRoot}/.htaccess" ) ) {
 				wfSuppressWarnings();
-				$ok = file_put_contents( "{$dirRoot}/.htaccess", "Deny from all\n" );
+				$ok = file_put_contents( "{$contRoot}/.htaccess", "Deny from all\n" );

It uses contRoot in a file exists call, and if it doesn't exist, attempting to create said file at that location makes sense..

#Comment by MarkAHershberger (talk | contribs)   15:45, 27 January 2012

Right, but my question was based on how $dir is set up: 

 $dir = ( $dirRel !=  ) ? "{$contRoot}/{$dirRel}" : $contRoot;

And the fact that index.html is checked for in $dir, not $contRoot.

Nevertheless, I think getting Aaron to check it (as you suggest) would be best.

(Maybe he could clarify $dir and $contRoot in the comments.)

#Comment by Aaron Schulz (talk | contribs)   18:31, 27 January 2012

The index file goes in the given dir, the access file goes in the container root. This looks OK.

#Comment by Reedy (talk | contribs)   15:40, 27 January 2012

Might just be easiest to get aaron to OK this one as he wrote the original code

Status & tagging log