Skip to content

Commit

Permalink
API CHANGE Don't reflect changes in File and Folder property setters …
Browse files Browse the repository at this point in the history 
…on filesystem before write() is called, to ensure that validate() applies in all cases. This fixes a problem where File->setName() would circumvent restrictions in File::$allowed_extensions (fixes #5693)

API CHANGE Removed File->resetFilename(), use File->updateFilesystem() to update the filesystem, and File->getRelativePath() to just update the "Filename" property without any filesystem changes (emulating the old $renamePhysicalFile method argument in resetFilename())
API CHANGE Removed File->autosetFilename(), please set the "Filename" property via File->getRelativePath()
MINOR Added unit tests to FileTest and FolderTest (some of them copied from FileTest, to test Folder behaviour separately)
ENHANCEMENT Added File::$allowed_extensions (backport from 2.4 to enable File->validate() security fix)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.3@108062 467b73ca-7a2a-4603-9d3b-597d59a354a9
  • Loading branch information
@chillu
chillu authored and Sam Minnee committed Feb 2, 2011
1 parent c129575 commit bdd30fa
Show file tree
Hide file tree
Showing 6 changed files with 605 additions and 131 deletions.
178 changes: 130 additions & 48 deletions filesystem/File.php
View file
Expand Up @@ -47,6 +47,26 @@ class File extends DataObject {
"Hierarchy", "Hierarchy",
); );


/**
* @var array List of allowed file extensions, enforced through {@link validate()}.
*/
public static $allowed_extensions = array(
'','html','htm','xhtml','js','css',
'bmp','png','gif','jpg','jpeg','ico','pcx','tif','tiff',
'au','mid','midi','mpa','mp3','ogg','m4a','ra','wma','wav','cda',
'avi','mpg','mpeg','asf','wmv','m4v','mov','mkv','mp4','swf','flv','ram','rm',
'doc','docx','txt','rtf','xls','xlsx','pages',
'ppt','pptx','pps','csv',
'cab','arj','tar','zip','zipx','sit','sitx','gz','tgz','bz2','ace','arc','pkg','dmg','hqx','jar',
'xml','pdf',
);

/**
* @var If this is true, then restrictions set in {@link $allowed_max_file_size} and
* {@link $allowed_extensions} will be applied to users with admin privileges as
* well.
*/
public static $apply_restrictions_to_admin = true;


/** /**
* Cached result of a "SHOW FIELDS" call * Cached result of a "SHOW FIELDS" call
Expand Down Expand Up @@ -99,7 +119,9 @@ function TreeTitle() {
protected function onBeforeDelete() { protected function onBeforeDelete() {
parent::onBeforeDelete(); parent::onBeforeDelete();


$this->autosetFilename(); // ensure that the record is synced with the filesystem before deleting
$this->updateFilesystem();

if($this->Filename && $this->Name && file_exists($this->getFullPath()) && !is_dir($this->getFullPath())) { if($this->Filename && $this->Name && file_exists($this->getFullPath()) && !is_dir($this->getFullPath())) {
unlink($this->getFullPath()); unlink($this->getFullPath());
} }
Expand Down Expand Up @@ -245,12 +267,17 @@ public function deleteDatabaseOnly() {
/** /**
* Event handler called before deleting from the database. * Event handler called before deleting from the database.
* You can overload this to clean up or otherwise process data before delete this * You can overload this to clean up or otherwise process data before delete this
* record. Don't forget to call parent::onBeforeDelete(), though! * record.
*/ */
protected function onBeforeWrite() { protected function onBeforeWrite() {
parent::onBeforeWrite(); parent::onBeforeWrite();


if(!$this->Name) $this->Name = "new-" . strtolower($this->class); // Set default name
if(!$this->getField('Name')) $this->Name = "new-" . strtolower($this->class);

// Set name on filesystem. If the current object is a "Folder", will also update references
// to subfolders and contained file records (both in database and filesystem)
$this->updateFilesystem();


if($brokenPages = $this->BackLinkTracking()) { if($brokenPages = $this->BackLinkTracking()) {
foreach($brokenPages as $brokenPage) { foreach($brokenPages as $brokenPage) {
Expand Down Expand Up @@ -311,56 +338,70 @@ function setName($name) {
} }
} }


// Update title
if(!$this->getField('Title')) $this->__set('Title', str_replace(array('-','_'),' ',ereg_replace('\.[^.]+$','',$name))); if(!$this->getField('Title')) $this->__set('Title', str_replace(array('-','_'),' ',ereg_replace('\.[^.]+$','',$name)));

// Update actual field value
$this->setField('Name', $name); $this->setField('Name', $name);



// Ensure that the filename is updated as well (only in-memory)
if($oldName && $oldName != $this->Name) { // Important: Circumvent the getter to avoid infinite loops
$this->resetFilename(); $this->setField('Filename', $this->getRelativePath());
} else {
$this->autosetFilename();
}



return $this->getField('Name'); return $this->getField('Name');
} }


/** /**
* Change a filename, moving the file if appropriate. * Moving the file if appropriate according to updated database content.
* @param $renamePhysicalFile Set this to false if you don't want to rename the physical file. Used when calling resetFilename() on the children of a folder. * Throws an Exception if the new file already exists.
*/ *
protected function resetFilename($renamePhysicalFile = true) { * Caution: This method should just be called during a {@link write()} invocation,
$oldFilename = $this->getField('Filename'); * as it relies on {@link DataObject->getChangedFields()}, which is reset after a {@link write()} call.
$newFilename = $this->getRelativePath(); * Might be called as {@link File->updateFilesystem()} from within {@link Folder->updateFilesystem()},

* so it has to handle both files and folders.
if($this->Name && $this->Filename && file_exists(Director::getAbsFile($oldFilename)) && strpos($oldFilename, '//') === false) { *
if($renamePhysicalFile) { * Assumes that the "Filename" property was previously updated, either directly or indirectly.
$from = Director::getAbsFile($oldFilename); * (it might have been influenced by {@link setName()} or {@link setParentID()} before).
$to = Director::getAbsFile($newFilename);

// Error checking
if(!file_exists($from)) user_error("Cannot move $oldFilename to $newFilename - $oldFilename doesn't exist", E_USER_WARNING);
else if(!file_exists(dirname($to))) user_error("Cannot move $oldFilename to $newFilename - " . dirname($newFilename) . " doesn't exist", E_USER_WARNING);
else if(!rename($from, $to)) user_error("Cannot move $oldFilename to $newFilename", E_USER_WARNING);

else $this->updateLinks($oldFilename, $newFilename);

} else {
$this->updateLinks($oldFilename, $newFilename);
}
} else {
// If the old file doesn't exist, maybe it's already been renamed.
if(file_exists(Director::getAbsFile($newFilename))) $this->updateLinks($oldFilename, $newFilename);
}

$this->setField('Filename', $newFilename);
}

/**
* Set the Filename field without manipulating the filesystem.
*/ */
protected function autosetFilename() { public function updateFilesystem() {
$changedFields = $this->getChangedFields();

// Regenerate "Filename", just to be sure
$this->setField('Filename', $this->getRelativePath()); $this->setField('Filename', $this->getRelativePath());

// If certain elements are changed, update the filesystem reference
if(!isset($changedFields['Filename'])) return false;

$pathBefore = $changedFields['Filename']['before'];
$pathAfter = $changedFields['Filename']['after'];

// If the file or folder didn't exist before, don't rename - its created
if(!$pathBefore) return;

$pathBeforeAbs = Director::getAbsFile($pathBefore);
$pathAfterAbs = Director::getAbsFile($pathAfter);

// Check that original file or folder exists, and rename on filesystem if required.
// The folder of the path might've already been renamed by Folder->updateFilesystem()
// before any filesystem update on contained file or subfolder records is triggered.
if(!file_exists($pathAfterAbs)) {
if(!is_a($this, 'Folder')) {
// Only throw a fatal error if *both* before and after paths don't exist.
if(!file_exists($pathBeforeAbs)) throw new Exception("Cannot move $pathBefore to $pathAfter - $pathBefore doesn't exist");

// Check that target directory (not the file itself) exists.
// Only check if we're dealing with a file, otherwise the folder will need to be created
if(!file_exists(dirname($pathAfterAbs))) throw new Exception("Cannot move $pathBefore to $pathAfter - Directory " . dirname($pathAfter) . " doesn't exist");
}

// Rename file or folder
$success = rename($pathBeforeAbs, $pathAfterAbs);
if(!$success) throw new Exception("Cannot move $pathBeforeAbs to $pathAfterAbs");
}


// Update any database references
$this->updateLinks($pathBeforeAbs, $pathAfterAbs);
} }


function setField( $field, $value ) { function setField( $field, $value ) {
Expand All @@ -385,11 +426,14 @@ protected function updateLinks($old, $new) {
} }
} }


/**
* Does not change the filesystem itself, please use {@link write()} for this.
*/
function setParentID($parentID) { function setParentID($parentID) {
$this->setField('ParentID', $parentID); $this->setField('ParentID', $parentID);


if($this->Name) $this->resetFilename(); // Don't change on the filesystem, we'll handle that in onBeforeWrite()
else $this->autosetFilename(); $this->setField('Filename', $this->getRelativePath());


return $this->getField('ParentID'); return $this->getField('ParentID');
} }
Expand Down Expand Up @@ -436,7 +480,7 @@ function getFullPath() {
function getRelativePath() { function getRelativePath() {


if($this->ParentID) { if($this->ParentID) {
$p = DataObject::get_one('Folder', "ID={$this->ParentID}"); $p = DataObject::get_by_id('Folder', $this->ParentID);


if($p && $p->ID) return $p->getRelativePath() . $this->getField("Name"); if($p && $p->ID) return $p->getRelativePath() . $this->getField("Name");
else return ASSETS_DIR . "/" . $this->getField("Name"); else return ASSETS_DIR . "/" . $this->getField("Name");
Expand All @@ -454,13 +498,17 @@ function DeleteLink() {
} }


function getFilename() { function getFilename() {
// Default behaviour: Return field if its set
if($this->getField('Filename')) { if($this->getField('Filename')) {
return $this->getField('Filename'); return $this->getField('Filename');
} else { } else {
return ASSETS_DIR . '/'; return ASSETS_DIR . '/';
} }
} }


/**
* Does not change the filesystem itself, please use {@link write()} for this.
*/
function setFilename($val) { function setFilename($val) {
$this->setField('Filename', $val); $this->setField('Filename', $val);
$this->setField('Name', basename($val)); $this->setField('Name', basename($val));
Expand All @@ -478,12 +526,19 @@ function getExtension() {
/** /**
* Gets the extension of a filepath or filename, * Gets the extension of a filepath or filename,
* by stripping away everything before the last "dot". * by stripping away everything before the last "dot".
* Caution: Only returns the last extension in "double-barrelled"
* extensions (e.g. "gz" for "tar.gz").
*
* Examples:
* - "myfile" returns ""
* - "myfile.txt" returns "txt"
* - "myfile.tar.gz" returns "gz"
* *
* @param string $filename * @param string $filename
* @return string * @return string
*/ */
public static function get_file_extension($filename) { public static function get_file_extension($filename) {
return strtolower(substr($filename,strrpos($filename,'.')+1)); return pathinfo($filename, PATHINFO_EXTENSION);
} }


/** /**
Expand Down Expand Up @@ -613,6 +668,33 @@ function fieldLabels($includerelations = true) {
return $labels; return $labels;
} }


function validate() {
if(File::$apply_restrictions_to_admin || !Permission::check('ADMIN')) {
// Extension validation
// TODO Merge this with Upload_Validator
$extension = $this->getExtension();
if($extension && !in_array($extension, self::$allowed_extensions)) {
$exts = self::$allowed_extensions;
sort($exts);
$message = sprintf(
_t(
'File.INVALIDEXTENSION',
'Extension is not allowed (valid: %s)',
PR_MEDIUM,
'Argument 1: Comma-separated list of valid extensions'
),
implode(', ',$exts)
);
return new ValidationResult(false, $message);
}
}

// We aren't validating for an existing "Filename" on the filesystem.
// A record should still be saveable even if the underlying record has been removed.

return new ValidationResult(true);
}

} }


?> ?>
2 changes: 1 addition & 1 deletion filesystem/Filesystem.php
View file
Expand Up @@ -51,7 +51,7 @@ public function fixfiles() {


$files = DataObject::get("File"); $files = DataObject::get("File");
foreach($files as $file) { foreach($files as $file) {
$file->resetFilename(); $file->updateFilesystem();
echo "<li>", $file->Filename; echo "<li>", $file->Filename;
$file->write(); $file->write();
} }
Expand Down
63 changes: 35 additions & 28 deletions filesystem/Folder.php
View file
Expand Up @@ -6,31 +6,41 @@
*/ */
class Folder extends File { class Folder extends File {


/* /**
* Find the given folder or create it, recursively. * Find the given folder or create it both as {@link Folder} database records
* and on the filesystem. If necessary, creates parent folders as well.
* *
* @param $folderPath string Absolute or relative path to the file * @param $folderPath string Absolute or relative path to the file.
* If path is relative, its interpreted relative to the "assets/" directory.
* @return Folder
*/ */
static function findOrMake($folderPath) { static function findOrMake($folderPath) {
// Create assets directory, if it is missing
if(!file_exists(ASSETS_PATH)) Filesystem::makeFolder(ASSETS_PATH);

$folderPath = trim(Director::makeRelative($folderPath)); $folderPath = trim(Director::makeRelative($folderPath));
// replace leading and trailing slashes // replace leading and trailing slashes
$folderPath = preg_replace('/^\/?(.*)\/?$/', '$1', $folderPath); $folderPath = preg_replace('/^\/?(.*)\/?$/', '$1', $folderPath);

$parts = explode("/",$folderPath); $parts = explode("/",$folderPath);
$parentID = 0;


$parentID = 0;
$item = null;
foreach($parts as $part) { foreach($parts as $part) {
$item = DataObject::get_one("Folder", "Name = '$part' AND ParentID = $parentID"); if(!$part) continue; // happens for paths with a trailing slash
$item = DataObject::get_one("Folder", "`Name` = '$part' AND `ParentID` = $parentID");
if(!$item) { if(!$item) {
$item = new Folder(); $item = new Folder();
$item->ParentID = $parentID; $item->ParentID = $parentID;
$item->Name = $part; $item->Name = $part;
$item->Title = $part; $item->Title = $part;
$item->write(); $item->write();
if(!file_exists($item->getFullPath())) mkdir($item->getFullPath(),Filesystem::$folder_create_mask); }
if(!file_exists($item->getFullPath())) {
Filesystem::makeFolder($item->getFullPath());
} }
$parentID = $item->ID; $parentID = $item->ID;
} }

return $item; return $item;
} }


Expand Down Expand Up @@ -201,6 +211,9 @@ function addUploadToFolder($tmpFile) {
} }
} }


function validate() {
return new ValidationResult(true);
}


//------------------------------------------------------------------------------------------------- //-------------------------------------------------------------------------------------------------
// Data Model Definition // Data Model Definition
Expand Down Expand Up @@ -261,37 +274,31 @@ public function myChildren() {
* Returns true if this folder has children * Returns true if this folder has children
*/ */
public function hasChildren() { public function hasChildren() {
return $this->ID && $this->myChildren() && $this->myChildren()->Count() > 0; return (bool)DB::query("SELECT COUNT(*) FROM `File` WHERE `ParentID` = "
. (int)$this->ID)->value();
} }

/** /**
* Overload autosetFilename() to call autosetFilename() on all the children, too * Returns true if this folder has children
*/ */
public function autosetFilename() { public function hasChildFolders() {
parent::autosetFilename(); $SQL_folderClasses = Convert::raw2sql(ClassInfo::subclassesFor('Folder'));


if($this->ID && ($children = $this->AllChildren())) { return (bool)DB::query("SELECT COUNT(*) FROM `File` WHERE `ParentID` = " . (int)$this->ID
$this->write(); . " AND `ClassName` IN ('" . implode("','", $SQL_folderClasses) . "')")->value();

foreach($children as $child) {
$child->autosetFilename();
$child->write();
}
}
} }


/** /**
* Overload resetFilename() to call resetFilename() on all the children, too. * Overloaded to call recursively on all contained {@link File} records.
* Pass renamePhysicalFile = false, since the folder renaming will have taken care of this
*/ */
protected function resetFilename($renamePhysicalFile = true) { public function updateFilesystem() {
parent::resetFilename($renamePhysicalFile); parent::updateFilesystem();


// Note: Folders will have been renamed on the filesystem already at this point,
// File->updateFilesystem() needs to take this into account.
if($this->ID && ($children = $this->AllChildren())) { if($this->ID && ($children = $this->AllChildren())) {
$this->write();

foreach($children as $child) { foreach($children as $child) {
$child->resetFilename(false); $child->updateFilesystem();
$child->write(); $child->write();
} }
} }
Expand Down

0 comments on commit bdd30fa

Please sign in to comment.