From a4a7a0e88fc07d06ea955274bcc75e8bf03cb078 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Sun, 8 Mar 2015 19:27:20 +0100 Subject: Unify file/cron and mfile->valid_id Create a testable function doing all the verification/removal, add tests and use it for both cases. Signed-off-by: Florian Pritz --- application/controllers/file.php | 43 ++++---- application/models/mfile.php | 43 ++++---- application/service/files.php | 50 ++++++++++ application/tests/test_service_files_valid_id.php | 114 ++++++++++++++++++++++ 4 files changed, 201 insertions(+), 49 deletions(-) create mode 100644 application/tests/test_service_files_valid_id.php (limited to 'application') diff --git a/application/controllers/file.php b/application/controllers/file.php index e258f2ffb..0dd6eaffe 100644 --- a/application/controllers/file.php +++ b/application/controllers/file.php @@ -966,39 +966,34 @@ class File extends MY_Controller { } } - // 0 age disables age checks - if ($this->config->item('upload_max_age') == 0) return; - $oldest_time = (time() - $this->config->item('upload_max_age')); $oldest_session_time = (time() - $this->config->item("sess_expiration")); + $config = array( + "upload_max_age" => $this->config->item("upload_max_age"), + "small_upload_size" => $this->config->item("small_upload_size"), + "sess_expiration" => $this->config->item("sess_expiration"), + ); + + $query = $this->db->select('hash, id, user, date') + ->from('files') + ->where("user", 0) + ->where("date <", $oldest_session_time) + ->get()->result_array(); + + foreach($query as $row) { + \service\files::valid_id($row, $config, $this->mfile, time()); + } - $small_upload_size = $this->config->item('small_upload_size'); + // 0 age disables age checks + if ($this->config->item('upload_max_age') == 0) return; - $query = $this->db->select('hash, id, user') + $query = $this->db->select('hash, id, user, date') ->from('files') ->where('date <', $oldest_time) - ->or_where('('.$this->db->_protect_identifiers('user').' = 0 AND ' - .$this->db->_protect_identifiers('date')." < $oldest_session_time)") ->get()->result_array(); foreach($query as $row) { - $file = $this->mfile->file($row['hash']); - if (!file_exists($file)) { - $this->mfile->delete_id($row["id"]); - continue; - } - - if ($row["user"] == 0 || filesize($file) > $small_upload_size) { - if (filemtime($file) < $oldest_time) { - unlink($file); - $this->mfile->delete_hash($row["hash"]); - } else { - $this->mfile->delete_id($row["id"]); - if ($this->mfile->stale_hash($row["hash"])) { - unlink($file); - } - } - } + \service\files::valid_id($row, $config, $this->mfile, time()); } } diff --git a/application/models/mfile.php b/application/models/mfile.php index f0a594609..bf6d0f9dc 100644 --- a/application/models/mfile.php +++ b/application/models/mfile.php @@ -130,36 +130,29 @@ class Mfile extends CI_Model { if (!$filedata) { return false; } - $file = $this->file($filedata['hash']); - if (!file_exists($file)) { - $this->delete_hash($filedata["hash"]); - return false; - } - - // 0 age disables age checks - if ($this->config->item('upload_max_age') == 0) return true; + $config = array( + "upload_max_age" => $this->config->item("upload_max_age"), + "small_upload_size" => $this->config->item("small_upload_size"), + "sess_expiration" => $this->config->item("sess_expiration"), + ); - // small files don't expire - if (filesize($file) <= $this->config->item("small_upload_size")) { - return true; - } + return \service\files::valid_id($filedata, $config, $this, time()); + } - // files older than this should be removed - $remove_before = (time()-$this->config->item('upload_max_age')); + public function file_exists($file) + { + return file_exists($file); + } - if ($filedata["date"] < $remove_before) { - // if the file has been uploaded multiple times the mtime is the time - // of the last upload - if (filemtime($file) < $remove_before) { - $this->delete_hash($filedata["hash"]); - } else { - $this->delete_id($id); - } - return false; - } + public function filemtime($file) + { + return filemtime($file); + } - return true; + public function filesize($file) + { + return filesize($file); } public function get_timeout($id) diff --git a/application/service/files.php b/application/service/files.php index e12e636be..1097c5201 100644 --- a/application/service/files.php +++ b/application/service/files.php @@ -281,4 +281,54 @@ class files { "url_id" => $url_id, ); } + + static public function valid_id(array $filedata, array $config, $model, $current_date) + { + assert(isset($filedata["hash"])); + assert(isset($filedata["id"])); + assert(isset($filedata["user"])); + assert(isset($filedata["date"])); + assert(isset($config["upload_max_age"])); + assert(isset($config["sess_expiration"])); + assert(isset($config["small_upload_size"])); + + $file = $model->file($filedata['hash']); + + if (!$model->file_exists($file)) { + $model->delete_hash($filedata["hash"]); + return false; + } + + if ($filedata["user"] == 0) { + if ($filedata["date"] < $current_date - $config["sess_expiration"]) { + $model->delete_id($filedata["id"]); + return false; + } + } + + // 0 age disables age checks + if ($config['upload_max_age'] == 0) return true; + + // small files don't expire + if ($model->filesize($file) <= $config["small_upload_size"]) { + return true; + } + + // files older than this should be removed + $remove_before = $current_date - $config["upload_max_age"]; + + if ($filedata["date"] < $remove_before) { + // if the file has been uploaded multiple times the mtime is the time + // of the last upload + $mtime = $model->filemtime($file); + if ($mtime < $remove_before) { + $model->delete_hash($filedata["hash"]); + } else { + $model->delete_id($filedata["id"]); + } + return false; + } + + return true; + } } diff --git a/application/tests/test_service_files_valid_id.php b/application/tests/test_service_files_valid_id.php new file mode 100644 index 000000000..436624336 --- /dev/null +++ b/application/tests/test_service_files_valid_id.php @@ -0,0 +1,114 @@ + + * + * Licensed under AGPLv3 + * (see COPYING for full license text) + * + */ + +namespace tests; + +class test_service_files_valid_id extends Test { + private $model; + private $filedata; + private $config; + + public function __construct() + { + parent::__construct(); + + $CI =& get_instance(); + $CI->load->model("muser"); + $CI->load->model("mfile"); + + } + + public function init() + { + $this->model = \Mockery::mock("Mfile"); + $this->model->shouldReceive("delete_id")->never()->byDefault(); + $this->model->shouldReceive("delete_hash")->never()->byDefault(); + $this->model->shouldReceive("file")->with("file-hash-1")->andReturn("/invalid/path/file-1")->byDefault(); + $this->model->shouldReceive("filemtime")->with("/invalid/path/file-1")->andReturn(500)->byDefault(); + $this->model->shouldReceive("filesize")->with("/invalid/path/file-1")->andReturn(50*1024)->byDefault(); + $this->model->shouldReceive("file_exists")->with("/invalid/path/file-1")->andReturn(true)->byDefault(); + + $this->filedata = array( + "hash" => "file-hash-1", + "id" => "file-id-1", + "user" => 2, + "date" => 500, + ); + + $this->config = array( + "upload_max_age" => 20, + "sess_expiration" => 10, + "small_upload_size" => 10*1024, + ); + } + + public function cleanup() + { + \Mockery::close(); + } + + public function test_valid_id_keepNormalUpload() + { + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 505); + $this->t->is($ret, true, "normal case should be valid"); + } + + public function test_valid_id_keepSmallUpload() + { + $this->model->shouldReceive("filesize")->with("/invalid/path/file-1")->once()->andReturn(50); + + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 550); + $this->t->is($ret, true, "file is old, but small and should be kept"); + } + + public function test_valid_id_removeOldFile() + { + $this->model->shouldReceive("delete_hash")->with("file-hash-1")->once(); + + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 550); + $this->t->is($ret, false, "file is old and should be removed"); + } + + public function test_valid_id_removeOldUpload() + { + $this->model->shouldReceive("delete_id")->with("file-id-1")->once(); + $this->model->shouldReceive("filemtime")->with("/invalid/path/file-1")->once()->andReturn(540); + + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 550); + $this->t->is($ret, false, "upload is old and should be removed"); + } + + public function test_valid_id_keepNormalUnownedFile() + { + $this->filedata["user"] = 0; + + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 505); + $this->t->is($ret, true, "upload is unowned and should be kept"); + } + + public function test_valid_id_removeOldUnownedFile() + { + $this->model->shouldReceive("delete_id")->with("file-id-1")->once(); + $this->filedata["user"] = 0; + + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 515); + $this->t->is($ret, false, "upload is old, unowned and should be removed"); + } + + public function test_valid_id_removeMissingFile() + { + $this->model->shouldReceive("file_exists")->with("/invalid/path/file-1")->once()->andReturn(false); + $this->model->shouldReceive("delete_hash")->with("file-hash-1")->once(); + + $ret = \service\files::valid_id($this->filedata, $this->config, $this->model, 505); + $this->t->is($ret, false, "missing file should be removed"); + } + +} + -- cgit v1.2.3-24-g4f1b