Permalink
Browse files

fix(file): ElggFile::delete() now removes target files if filename is…

… a symlink

By default, ElggFile::delete() now also deletes the target of a symbolic link,
if the filename is a link.
  • Loading branch information...
hypeJunction committed Apr 9, 2016
1 parent 186a355 commit facc13fe13f8719204c85b7f7334acfa5e8f06da
@@ -144,13 +144,17 @@ public function close($f) {
/**
* Delete an \ElggFile file.
*
* @param \ElggFile $file File to delete
*
* @param \ElggFile $file File to delete
* @param bool $follow_symlinks If true, will also delete the target file if the current file is a symlink
* @return bool
*/
public function delete(\ElggFile $file) {
public function delete(\ElggFile $file, $follow_symlinks = true) {
$filename = $this->getFilenameOnFilestore($file);
if (file_exists($filename)) {
if (file_exists($filename) || is_link($filename)) {
if ($follow_symlinks && is_link($filename) && file_exists($filename)) {
$target = readlink($filename);
file_exists($target) && unlink($target);
}
return unlink($filename);
} else {
return true;
@@ -294,12 +294,13 @@ public function close() {
/**
* Delete this file.
*
* @param bool $follow_symlinks If true, will also delete the target file if the current file is a symlink
* @return bool
*/
public function delete() {
public function delete($follow_symlinks = true) {
$fs = $this->getFilestore();
$result = $fs->delete($this);
$result = $fs->delete($this, $follow_symlinks);
if ($this->getGUID() && $result) {
$result = parent::delete();
@@ -78,11 +78,11 @@
/**
* Delete the file associated with a given file handle.
*
* @param \ElggFile $file The file
*
* @param \ElggFile $file The file
* @param bool $follow_symlinks If true, will also delete the target file if the current file is a symlink
* @return bool
*/
abstract public function delete(\ElggFile $file);
abstract public function delete(\ElggFile $file, $follow_symlinks = true);
/**
* Return the size in bytes for a given file.
@@ -250,11 +250,18 @@ public function testCanCreateAndReadSymlinks() {
unlink("$dataroot$dir$symlink_name");
}
$target = new ElggFile();
$target->owner_guid = 1;
$target->setFilename('symlink-target.txt');
$target->open('write');
$target->write('Testing!');
$target->close();
$symlink = new ElggFile();
$symlink->owner_guid = 1;
$symlink->setFilename($symlink_name);
$to = $this->file->getFilenameOnFilestore();
$to = $target->getFilenameOnFilestore();
$from = $symlink->getFilenameOnFilestore();
$this->assertTrue(symlink($to, $from));
@@ -263,9 +270,9 @@ public function testCanCreateAndReadSymlinks() {
$this->assertTrue($symlink->exists());
$this->file->open('read');
$file_contents = $this->file->grabFile();
$this->file->close();
$target->open('read');
$file_contents = $target->grabFile();
$target->close();
$symlink->open('read');
$symlink_contents = $symlink->grabFile();
@@ -279,11 +286,81 @@ public function testCanCreateAndReadSymlinks() {
/**
* @group FileService
* @todo ElggFile::delete() does not follow symlinks. Once fixed, we need to test
* that when symlink is deleted, we also delete the target file
*/
public function testCanDeleteSymlinkTarget() {
$this->markTestIncomplete();
public function testCanDeleteSymlinkAndKeepTarget() {
$to = new ElggFile();
$to->owner_guid = 1;
$to->setFilename('symlink-target.txt');
$to->open('write');
$to->close();
$from = new ElggFile();
$from->owner_guid = 1;
$from->setFilename('symlink.txt');
$to_filename = $to->getFilenameOnFilestore();
$from_filename = $from->getFilenameOnFilestore();
// Delete the symlink but keep the target
$this->assertTrue(symlink($to_filename, $from_filename));
$this->assertTrue($from->delete(false));
$this->assertFalse($from->exists());
$this->assertFalse(is_link($from_filename));
$this->assertTrue($to->exists());
}
/**
* @group FileService
*/
public function testCanDeleteSymlinkAndTarget() {
$to = new ElggFile();
$to->owner_guid = 1;
$to->setFilename('symlink-target.txt');
$to->open('write');
$to->close();
$from = new ElggFile();
$from->owner_guid = 1;
$from->setFilename('symlink.txt');
$to_filename = $to->getFilenameOnFilestore();
$from_filename = $from->getFilenameOnFilestore();
// Delete the symlink and the target
$this->assertTrue(symlink($to_filename, $from_filename));
$this->assertTrue($from->delete(true));
$this->assertFalse($from->exists());
$this->assertFalse(is_link($from_filename));
$this->assertfalse($to->exists());
}
/**
* @group FileService
*/
public function testCanDeleteSymlinkWithMissingTarget() {
$to = new ElggFile();
$to->owner_guid = 1;
$to->setFilename('symlink-target.txt');
$to->open('write');
$to->close();
$from = new ElggFile();
$from->owner_guid = 1;
$from->setFilename('symlink.txt');
$to_filename = $to->getFilenameOnFilestore();
$from_filename = $from->getFilenameOnFilestore();
// Test there are no errors when target doesn't exist anymore
$this->assertTrue(symlink($to_filename, $from_filename));
$this->assertTrue($to->delete());
$this->assertTrue($from->delete(true));
$this->assertFalse($from->exists());
$this->assertFalse(is_link($from_filename));
$this->assertFalse($to->exists());
}
}
@@ -1 +1 @@
Hello world!
Hello world!

0 comments on commit facc13f

Please sign in to comment.