From 31ba081b793141ca36866a6dd349cd2eac5af68e Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Thu, 21 Apr 2011 02:06:53 -0600 Subject: Add an event that will collect all valid filename extensions. --- modules/gallery/controllers/uploader.php | 2 +- modules/gallery/libraries/Form_Uploadify.php | 1 + modules/gallery/models/item.php | 9 +++++---- modules/gallery/views/form_uploadify.html.php | 2 +- system/helpers/upload.php | 20 ++++++++++++++++++++ 5 files changed, 28 insertions(+), 6 deletions(-) diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php index 6b1455e4..12180893 100644 --- a/modules/gallery/controllers/uploader.php +++ b/modules/gallery/controllers/uploader.php @@ -51,7 +51,7 @@ class Uploader_Controller extends Controller { $file_validation = new Validation($_FILES); $file_validation->add_rules( "Filedata", "upload::valid", "upload::required", - "upload::type[gif,jpg,jpeg,png,flv,mp4,m4v]"); + "upload::type[" . implode(",", upload::get_upload_extensions()) . "]"); if ($form->validate() && $file_validation->validate()) { $temp_filename = upload::save("Filedata"); diff --git a/modules/gallery/libraries/Form_Uploadify.php b/modules/gallery/libraries/Form_Uploadify.php index 27ab9684..ca189f0b 100644 --- a/modules/gallery/libraries/Form_Uploadify.php +++ b/modules/gallery/libraries/Form_Uploadify.php @@ -47,6 +47,7 @@ class Form_Uploadify_Core extends Form_Input { $v->script_data = $this->data["script_data"]; $v->simultaneous_upload_limit = module::get_var("gallery", "simultaneous_upload_limit"); $v->movies_allowed = (bool) movie::find_ffmpeg(); + $v->extensions = upload::get_upload_filters(); $v->suhosin_session_encrypt = (bool) ini_get("suhosin.session.encrypt"); return $v; } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 8f4bc5e4..aaca832a 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -787,9 +787,10 @@ class Item_Model_Core extends ORM_MPTT { return; } - if ($this->is_movie() && !preg_match("/^(flv|mp4|m4v)$/i", $ext)) { - $v->add_error("name", "illegal_data_file_extension"); - } else if ($this->is_photo() && !preg_match("/^(gif|jpg|jpeg|png)$/i", $ext)) { + if (($this->is_movie() || $this->is_photo()) && + !preg_match("/^(" . + implode("|", array_map("preg_quote", upload::get_upload_extensions())) . + ")\$/i", $ext)) { $v->add_error("name", "illegal_data_file_extension"); } } @@ -881,7 +882,7 @@ class Item_Model_Core extends ORM_MPTT { if ($this->is_movie()) { $legal_values = array("video/flv", "video/x-flv", "video/mp4"); } if ($this->is_photo()) { - $legal_values = array("image/jpeg", "image/gif", "image/png"); + $legal_values = array("image/jpeg", "image/gif", "image/png", "image/tiff"); } break; diff --git a/modules/gallery/views/form_uploadify.html.php b/modules/gallery/views/form_uploadify.html.php index 77b6d493..db90b733 100644 --- a/modules/gallery/views/form_uploadify.html.php +++ b/modules/gallery/views/form_uploadify.html.php @@ -28,7 +28,7 @@ uploader: "", script: "id}") ?>", scriptData: , - fileExt: "*.gif;*.jpg;*.jpeg;*.png;*.GIF;*.JPG;*.JPEG;*.PNG;*.flv;*.mp4;*.m4v;*.FLV;*.MP4;*.M4V", + fileExt: "", fileDesc: for_js() ?>, cancelImg: "", simUploadLimit: , diff --git a/system/helpers/upload.php b/system/helpers/upload.php index 62de674f..cfd92dd1 100644 --- a/system/helpers/upload.php +++ b/system/helpers/upload.php @@ -154,4 +154,24 @@ class upload_Core { return ($file['size'] <= $size); } + + static function get_upload_extensions() { + // Create a default list of allowed extensions and then let modules modify it. + $extensions_wrapper = new stdClass(); + $extensions_wrapper->extensions = array("gif", "jpg", "jpeg", "png"); + if (movie::find_ffmpeg()) { + array_push($extensions_wrapper->extensions, "flv", "mp4", "m4v"); + } + module::event("upload_extensions", $extensions_wrapper); + return $extensions_wrapper->extensions; + } + + static function get_upload_filters() { + $filters = array(); + foreach (upload::get_upload_extensions() as $extension) { + array_push($filters, "*." . $extension, "*." . strtoupper($extension)); + } + return $filters; + } + } // End upload \ No newline at end of file -- cgit v1.2.3 From 567522bfa08c370bb5baf8454afc5b04bc9e49b4 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Thu, 21 Apr 2011 20:12:32 -0600 Subject: Add an event for when a new graphics toolkit is chosen. --- modules/gallery/controllers/admin_graphics.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/gallery/controllers/admin_graphics.php b/modules/gallery/controllers/admin_graphics.php index a2d19d4a..a8a7cdc0 100644 --- a/modules/gallery/controllers/admin_graphics.php +++ b/modules/gallery/controllers/admin_graphics.php @@ -40,6 +40,8 @@ class Admin_Graphics_Controller extends Admin_Controller { $msg = t("Changed graphics toolkit to: %toolkit", array("toolkit" => $tk->$toolkit_id->name)); message::success($msg); log::success("graphics", $msg); + + module::event("graphics_toolkit_change", $toolkit_id); } url::redirect("admin/graphics"); -- cgit v1.2.3 From 6702104f571413e4d57db3515b2070c48d3e9b55 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 23 Apr 2011 16:35:00 -0600 Subject: Resolve an infinite recursion that happens when the path caches are updated during saving. --- modules/gallery/models/item.php | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index aaca832a..a8bca15c 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -432,6 +432,7 @@ class Item_Model_Core extends ORM_MPTT { if ($original->parent_id != $this->parent_id || $original->name != $this->name) { // Move all of the items associated data files + $this->_build_relative_caches(); @rename($original->file_path(), $this->file_path()); if ($this->is_album()) { @rename(dirname($original->resize_path()), dirname($this->resize_path())); -- cgit v1.2.3 From e149cf7238a1f8eaddfc68580f2d636dd8255795 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 23 Apr 2011 16:39:25 -0600 Subject: Support data files that change their extension and MIME type. --- modules/gallery/models/item.php | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index a8bca15c..299d3584 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -410,6 +410,15 @@ class Item_Model_Core extends ORM_MPTT { // If any significant fields have changed, load up a copy of the original item and // keep it around. $original = ORM::factory("item", $this->id); + + // Preserve the extension of the data file. + if (isset($this->data_file)) { + $extension = pathinfo($this->data_file, PATHINFO_EXTENSION); + if (!empty($extension)) { + $this->name = pathinfo($this->name, PATHINFO_FILENAME) . ".$extension"; + } + } + if (array_intersect($this->changed, array("parent_id", "name", "slug"))) { $original->_build_relative_caches(); $this->relative_path_cache = null; @@ -463,8 +472,6 @@ class Item_Model_Core extends ORM_MPTT { } // Replace the data file, if requested. - // @todo: we don't handle the case where you swap in a file of a different mime type - // should we prevent that in validation? or in set_data_file() if ($this->data_file && ($this->is_photo() || $this->is_movie())) { copy($this->data_file, $this->file_path()); @@ -520,6 +527,8 @@ class Item_Model_Core extends ORM_MPTT { $this->name = "$base_name-$rand"; } $this->slug = "$base_slug-$rand"; + $this->relative_path_cache = null; + $this->relative_url_cache = null; } } @@ -771,16 +780,7 @@ class Item_Model_Core extends ORM_MPTT { } if ($this->is_movie() || $this->is_photo()) { - if ($this->loaded()) { - // Existing items can't change their extension - $original = ORM::factory("item", $this->id); - $new_ext = pathinfo($this->name, PATHINFO_EXTENSION); - $old_ext = pathinfo($original->name, PATHINFO_EXTENSION); - if (strcasecmp($new_ext, $old_ext)) { - $v->add_error("name", "illegal_data_file_extension"); - return; - } - } else { + if (!$this->loaded()) { // New items must have an extension $ext = pathinfo($this->name, PATHINFO_EXTENSION); if (!$ext) { @@ -817,17 +817,6 @@ class Item_Model_Core extends ORM_MPTT { } else if (filesize($this->data_file) == 0) { $v->add_error("name", "empty_data_file"); } - - if ($this->loaded()) { - if ($this->is_photo()) { - list ($a, $b, $mime_type) = photo::get_file_metadata($this->data_file); - } else if ($this->is_movie()) { - list ($a, $b, $mime_type) = movie::get_file_metadata($this->data_file); - } - if ($mime_type != $this->mime_type) { - $v->add_error("name", "cant_change_mime_type"); - } - } } /** -- cgit v1.2.3 From 0d6a3a3cfc4f38f450db9e18da47a5e2ad826af8 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 23 Apr 2011 21:19:47 -0600 Subject: Create a tempnam substitute that safely creates files with a given extension. --- modules/gallery/controllers/quick.php | 4 +-- modules/gallery/helpers/system.php | 25 ++++++++++++++ modules/gallery/tests/Mock_Built_In.php | 39 ++++++++++++++++++++++ modules/gallery/tests/System_Helper_Test.php | 49 ++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 modules/gallery/tests/Mock_Built_In.php create mode 100644 modules/gallery/tests/System_Helper_Test.php diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index da4768fd..ce52cb8d 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -36,8 +36,8 @@ class Quick_Controller extends Controller { } if ($degrees) { - $tmpfile = tempnam(TMPPATH, "rotate") . "." . - pathinfo($item->file_path(), PATHINFO_EXTENSION); + $tmpfile = system::tempnam(TMPPATH, "rotate", + "." . pathinfo($item->file_path(), PATHINFO_EXTENSION)); gallery_graphics::rotate($item->file_path(), $tmpfile, array("degrees" => $degrees), $item); $item->set_data_file($tmpfile); $item->save(); diff --git a/modules/gallery/helpers/system.php b/modules/gallery/helpers/system.php index c39c7227..31ecafa7 100644 --- a/modules/gallery/helpers/system.php +++ b/modules/gallery/helpers/system.php @@ -40,4 +40,29 @@ class system_Core { } return null; } + + /** + * Create a file with a unique file name. + * This helper is similar to the built-in tempnam, except that it supports an optional postfix. + */ + static function tempnam($dir = TMPPATH, $prefix = "", $postfix = "") { + return self::_tempnam($dir, $prefix, $postfix, "tempnam"); + } + + // This helper provides a dependency-injected implementation of tempnam. + static function _tempnam($dir, $prefix, $postfix, $builtin) { + $success = false; + do { + $basename = call_user_func($builtin, $dir, $prefix); + if (!$basename) { + return false; + } + $filename = $basename . $postfix; + $success = !file_exists($filename) && @rename($basename, $filename); + if (!$success) { + @unlink($basename); + } + } while (!$success); + return $filename; + } } \ No newline at end of file diff --git a/modules/gallery/tests/Mock_Built_In.php b/modules/gallery/tests/Mock_Built_In.php new file mode 100644 index 00000000..b02e5ecf --- /dev/null +++ b/modules/gallery/tests/Mock_Built_In.php @@ -0,0 +1,39 @@ +nonces = func_get_args(); + } + + function _tempnam($dir, $prefix) { + if (empty($this->nonces)) + return false; + $filename = "$dir/$prefix" . array_shift($this->nonces); + if (!touch($filename)) + return false; + return $filename; + } +} diff --git a/modules/gallery/tests/System_Helper_Test.php b/modules/gallery/tests/System_Helper_Test.php new file mode 100644 index 00000000..734f98ac --- /dev/null +++ b/modules/gallery/tests/System_Helper_Test.php @@ -0,0 +1,49 @@ +assert_true(file_exists($filename), "File not created"); + unlink($filename); + } + + public function tempnam_collision_test() { + require_once('Mock_Built_In.php'); + $existing = TMPPATH . "/file1.ext"; + $available = TMPPATH . "/file2.ext"; + touch($existing); + $filename = system::_tempnam(TMPPATH, "file", ".ext", + array(new Mock_Built_In("1", "2"), "_tempnam")); + unlink($existing); + $this->assert_true(file_exists($filename), "File not created"); + unlink($filename); + $this->assert_equal($available, $filename, "Incorrect filename created"); + } + + public function tempnam_abort_test() { + require_once('Mock_Built_In.php'); + $filename = system::_tempnam(TMPPATH, "file", ".ext", + array(new Mock_Built_In(), "_tempnam")); + if ($filename) { + @unlink($filename); + } + $this->assert_false($filename, "Operation not aborted"); + } +} -- cgit v1.2.3 From c6ef706d70c7e48bea1145eec1b13fb5683e023f Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 23 Apr 2011 22:55:59 -0600 Subject: Preserve old data files long enough for them to be available to event handlers. --- modules/gallery/models/item.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 299d3584..482b6247 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -442,7 +442,9 @@ class Item_Model_Core extends ORM_MPTT { if ($original->parent_id != $this->parent_id || $original->name != $this->name) { // Move all of the items associated data files $this->_build_relative_caches(); - @rename($original->file_path(), $this->file_path()); + if (!isset($this->data_file)) { + @rename($original->file_path(), $this->file_path()); + } if ($this->is_album()) { @rename(dirname($original->resize_path()), dirname($this->resize_path())); @rename(dirname($original->thumb_path()), dirname($this->thumb_path())); @@ -491,6 +493,9 @@ class Item_Model_Core extends ORM_MPTT { // Null out the data file variable here, otherwise this event will trigger another // save() which will think that we're doing another file move. $this->data_file = null; + if ($original->file_path() != $this->file_path()) { + @unlink($original->file_path()); + } module::event("item_updated_data_file", $this); } } -- cgit v1.2.3 From fcb06bf175bb9eeff36d9c294e97ace9374ef0f3 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sun, 24 Apr 2011 00:45:12 -0600 Subject: Don't assign to the item->name field if the name is unchanged, because the save method will crash. --- modules/gallery/models/item.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 482b6247..7a08d9c2 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -414,8 +414,9 @@ class Item_Model_Core extends ORM_MPTT { // Preserve the extension of the data file. if (isset($this->data_file)) { $extension = pathinfo($this->data_file, PATHINFO_EXTENSION); - if (!empty($extension)) { - $this->name = pathinfo($this->name, PATHINFO_FILENAME) . ".$extension"; + $new_name = pathinfo($this->name, PATHINFO_FILENAME) . ".$extension"; + if (!empty($extension) && strcmp($this->name, $new_name)) { + $this->name = $new_name; } } -- cgit v1.2.3 From 809567f12850f59bdeb47a2963f6968b99b5a201 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sun, 24 Apr 2011 08:10:04 -0600 Subject: Expose the data file field. --- modules/gallery/models/item.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 7a08d9c2..f4d4c521 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -127,6 +127,15 @@ class Item_Model_Core extends ORM_MPTT { return $this; } + /** + * Get the path to the data file associated with this item. + * This data file field is only set until you call save(). + * After that, you can get the path using get_file_path(). + */ + public function get_data_file() { + return $this->data_file; + } + /** * Return the server-relative url to this item, eg: * /gallery3/index.php/BobsWedding?page=2 -- cgit v1.2.3 From 7ff485fa48c392bbbb0370f67cb1bd6fcc00c2a4 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 27 Apr 2011 20:29:06 -0600 Subject: Move the extensions helpers out of the Kohana system directory and into their own Gallery Extensions class. --- modules/gallery/controllers/uploader.php | 2 +- modules/gallery/helpers/extensions.php | 39 ++++++++++++++++++++++++++++ modules/gallery/libraries/Form_Uploadify.php | 4 +-- modules/gallery/models/item.php | 3 ++- system/helpers/upload.php | 20 -------------- 5 files changed, 44 insertions(+), 24 deletions(-) create mode 100644 modules/gallery/helpers/extensions.php diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php index 12180893..5f3e9ca4 100644 --- a/modules/gallery/controllers/uploader.php +++ b/modules/gallery/controllers/uploader.php @@ -51,7 +51,7 @@ class Uploader_Controller extends Controller { $file_validation = new Validation($_FILES); $file_validation->add_rules( "Filedata", "upload::valid", "upload::required", - "upload::type[" . implode(",", upload::get_upload_extensions()) . "]"); + "upload::type[" . implode(",", extensions::get_upload_extensions()) . "]"); if ($form->validate() && $file_validation->validate()) { $temp_filename = upload::save("Filedata"); diff --git a/modules/gallery/helpers/extensions.php b/modules/gallery/helpers/extensions.php new file mode 100644 index 00000000..bccbfc41 --- /dev/null +++ b/modules/gallery/helpers/extensions.php @@ -0,0 +1,39 @@ +extensions = array("gif", "jpg", "jpeg", "png"); + if (movie::find_ffmpeg()) { + array_push($extensions_wrapper->extensions, "flv", "mp4", "m4v"); + } + module::event("upload_extensions", $extensions_wrapper); + return $extensions_wrapper->extensions; + } + + static function get_upload_filters() { + $filters = array(); + foreach (self::get_upload_extensions() as $extension) { + array_push($filters, "*." . $extension, "*." . strtoupper($extension)); + } + return $filters; + } +} diff --git a/modules/gallery/libraries/Form_Uploadify.php b/modules/gallery/libraries/Form_Uploadify.php index c79b9aa2..c046fe3a 100644 --- a/modules/gallery/libraries/Form_Uploadify.php +++ b/modules/gallery/libraries/Form_Uploadify.php @@ -47,7 +47,7 @@ class Form_Uploadify_Core extends Form_Input { $v->script_data = $this->data["script_data"]; $v->simultaneous_upload_limit = module::get_var("gallery", "simultaneous_upload_limit"); $v->movies_allowed = (bool) movie::find_ffmpeg(); - $v->extensions = upload::get_upload_filters(); + $v->extensions = extensions::get_upload_filters(); $v->suhosin_session_encrypt = (bool) ini_get("suhosin.session.encrypt"); list ($toolkit_max_filesize_bytes, $toolkit_max_filesize) = graphics::max_filesize(); @@ -69,4 +69,4 @@ class Form_Uploadify_Core extends Form_Input { public function validate() { return true; } -} \ No newline at end of file +} diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index ba8e7cde..dcdcfd0d 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -803,7 +803,8 @@ class Item_Model_Core extends ORM_MPTT { if (($this->is_movie() || $this->is_photo()) && !preg_match("/^(" . - implode("|", array_map("preg_quote", upload::get_upload_extensions())) . + implode("|", array_map("preg_quote", + extensions::get_upload_extensions())) . ")\$/i", $ext)) { $v->add_error("name", "illegal_data_file_extension"); } diff --git a/system/helpers/upload.php b/system/helpers/upload.php index cfd92dd1..62de674f 100644 --- a/system/helpers/upload.php +++ b/system/helpers/upload.php @@ -154,24 +154,4 @@ class upload_Core { return ($file['size'] <= $size); } - - static function get_upload_extensions() { - // Create a default list of allowed extensions and then let modules modify it. - $extensions_wrapper = new stdClass(); - $extensions_wrapper->extensions = array("gif", "jpg", "jpeg", "png"); - if (movie::find_ffmpeg()) { - array_push($extensions_wrapper->extensions, "flv", "mp4", "m4v"); - } - module::event("upload_extensions", $extensions_wrapper); - return $extensions_wrapper->extensions; - } - - static function get_upload_filters() { - $filters = array(); - foreach (upload::get_upload_extensions() as $extension) { - array_push($filters, "*." . $extension, "*." . strtoupper($extension)); - } - return $filters; - } - } // End upload \ No newline at end of file -- cgit v1.2.3 From 4c2b2ebd3f2052898fbfb175650ed4cf49c8006e Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 27 Apr 2011 20:52:35 -0600 Subject: Remove a newline at the end of the file that I accidentally introduced. --- modules/gallery/libraries/Form_Uploadify.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gallery/libraries/Form_Uploadify.php b/modules/gallery/libraries/Form_Uploadify.php index c046fe3a..884653e2 100644 --- a/modules/gallery/libraries/Form_Uploadify.php +++ b/modules/gallery/libraries/Form_Uploadify.php @@ -69,4 +69,4 @@ class Form_Uploadify_Core extends Form_Input { public function validate() { return true; } -} +} \ No newline at end of file -- cgit v1.2.3 From 7e61a01a96f5eab7212dba754ac64fdfb4d9e8ab Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 30 Apr 2011 16:08:49 -0600 Subject: Change the name of the extensions helper to legal_file. --- modules/gallery/helpers/legal_file.php | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 modules/gallery/helpers/legal_file.php diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php new file mode 100644 index 00000000..2cb0fb25 --- /dev/null +++ b/modules/gallery/helpers/legal_file.php @@ -0,0 +1,39 @@ +extensions = array("gif", "jpg", "jpeg", "png"); + if (movie::find_ffmpeg()) { + array_push($extensions_wrapper->extensions, "flv", "mp4", "m4v"); + } + module::event("legal_file_extensions", $extensions_wrapper); + return $extensions_wrapper->extensions; + } + + static function get_filters() { + $filters = array(); + foreach (self::get_extensions() as $extension) { + array_push($filters, "*." . $extension, "*." . strtoupper($extension)); + } + return $filters; + } +} -- cgit v1.2.3 From a8ca9dcf9edd54633c0c78b3af76aa974d38fc64 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 30 Apr 2011 16:10:06 -0600 Subject: Change the name of the extensions helper to legal_file. --- modules/gallery/controllers/uploader.php | 2 +- modules/gallery/helpers/extensions.php | 39 ---------------------------- modules/gallery/libraries/Form_Uploadify.php | 2 +- modules/gallery/models/item.php | 2 +- 4 files changed, 3 insertions(+), 42 deletions(-) delete mode 100644 modules/gallery/helpers/extensions.php diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php index 5f3e9ca4..9c2bf7d7 100644 --- a/modules/gallery/controllers/uploader.php +++ b/modules/gallery/controllers/uploader.php @@ -51,7 +51,7 @@ class Uploader_Controller extends Controller { $file_validation = new Validation($_FILES); $file_validation->add_rules( "Filedata", "upload::valid", "upload::required", - "upload::type[" . implode(",", extensions::get_upload_extensions()) . "]"); + "upload::type[" . implode(",", legal_file::get_extensions()) . "]"); if ($form->validate() && $file_validation->validate()) { $temp_filename = upload::save("Filedata"); diff --git a/modules/gallery/helpers/extensions.php b/modules/gallery/helpers/extensions.php deleted file mode 100644 index bccbfc41..00000000 --- a/modules/gallery/helpers/extensions.php +++ /dev/null @@ -1,39 +0,0 @@ -extensions = array("gif", "jpg", "jpeg", "png"); - if (movie::find_ffmpeg()) { - array_push($extensions_wrapper->extensions, "flv", "mp4", "m4v"); - } - module::event("upload_extensions", $extensions_wrapper); - return $extensions_wrapper->extensions; - } - - static function get_upload_filters() { - $filters = array(); - foreach (self::get_upload_extensions() as $extension) { - array_push($filters, "*." . $extension, "*." . strtoupper($extension)); - } - return $filters; - } -} diff --git a/modules/gallery/libraries/Form_Uploadify.php b/modules/gallery/libraries/Form_Uploadify.php index 884653e2..450320b3 100644 --- a/modules/gallery/libraries/Form_Uploadify.php +++ b/modules/gallery/libraries/Form_Uploadify.php @@ -47,7 +47,7 @@ class Form_Uploadify_Core extends Form_Input { $v->script_data = $this->data["script_data"]; $v->simultaneous_upload_limit = module::get_var("gallery", "simultaneous_upload_limit"); $v->movies_allowed = (bool) movie::find_ffmpeg(); - $v->extensions = extensions::get_upload_filters(); + $v->extensions = legal_file::get_filters(); $v->suhosin_session_encrypt = (bool) ini_get("suhosin.session.encrypt"); list ($toolkit_max_filesize_bytes, $toolkit_max_filesize) = graphics::max_filesize(); diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 5ccbe75c..27b821f3 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -804,7 +804,7 @@ class Item_Model_Core extends ORM_MPTT { if (($this->is_movie() || $this->is_photo()) && !preg_match("/^(" . implode("|", array_map("preg_quote", - extensions::get_upload_extensions())) . + legal_file::get_extensions())) . ")\$/i", $ext)) { $v->add_error("name", "illegal_data_file_extension"); } -- cgit v1.2.3 From 2375a02e2cdbd1ccaf7dc4d3db9d85119972e3a9 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 30 Apr 2011 16:40:55 -0600 Subject: Change the signature of system::tempnam to something more appropriate for Gallery. --- modules/gallery/controllers/quick.php | 4 ++-- modules/gallery/helpers/system.php | 13 ++++++++----- modules/gallery/tests/System_Helper_Test.php | 5 +++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index ce52cb8d..b6576ec0 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -36,8 +36,8 @@ class Quick_Controller extends Controller { } if ($degrees) { - $tmpfile = system::tempnam(TMPPATH, "rotate", - "." . pathinfo($item->file_path(), PATHINFO_EXTENSION)); + $tmpfile = system::temp_filename("rotate", + pathinfo($item->file_path(), PATHINFO_EXTENSION)); gallery_graphics::rotate($item->file_path(), $tmpfile, array("degrees" => $degrees), $item); $item->set_data_file($tmpfile); $item->save(); diff --git a/modules/gallery/helpers/system.php b/modules/gallery/helpers/system.php index 31ecafa7..9815d588 100644 --- a/modules/gallery/helpers/system.php +++ b/modules/gallery/helpers/system.php @@ -43,15 +43,18 @@ class system_Core { /** * Create a file with a unique file name. - * This helper is similar to the built-in tempnam, except that it supports an optional postfix. + * This helper is similar to the built-in tempnam. + * It allows the caller to specify a prefix and an extension. + * It always places the file in TMPPATH. */ - static function tempnam($dir = TMPPATH, $prefix = "", $postfix = "") { - return self::_tempnam($dir, $prefix, $postfix, "tempnam"); + static function temp_filename($prefix = "", $extension = "") { + return self::_tempnam(TMPPATH, $prefix, ".$extension", "tempnam"); } - // This helper provides a dependency-injected implementation of tempnam. + /** + * This helper provides a dependency-injected implementation of tempnam. + */ static function _tempnam($dir, $prefix, $postfix, $builtin) { - $success = false; do { $basename = call_user_func($builtin, $dir, $prefix); if (!$basename) { diff --git a/modules/gallery/tests/System_Helper_Test.php b/modules/gallery/tests/System_Helper_Test.php index 734f98ac..dfe5d9ab 100644 --- a/modules/gallery/tests/System_Helper_Test.php +++ b/modules/gallery/tests/System_Helper_Test.php @@ -18,10 +18,11 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class System_Helper_Test extends Gallery_Unit_Test_Case { - public function tempnam_random_test() { - $filename = system::tempnam(TMPPATH, "file", ".ext"); + public function temp_filename_random_test() { + $filename = system::temp_filename("file", "ext"); $this->assert_true(file_exists($filename), "File not created"); unlink($filename); + $this->assert_pattern($filename, "|/file.*\\.ext$|"); } public function tempnam_collision_test() { -- cgit v1.2.3 From c3e8c1e3b5e3cb1046acd4c923bb0ae9dbcd603a Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 30 Apr 2011 18:12:56 -0600 Subject: The data_file field is public, so we don't need to supply an accessor method. --- modules/gallery/models/item.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 27b821f3..22634cbf 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -127,15 +127,6 @@ class Item_Model_Core extends ORM_MPTT { return $this; } - /** - * Get the path to the data file associated with this item. - * This data file field is only set until you call save(). - * After that, you can get the path using get_file_path(). - */ - public function get_data_file() { - return $this->data_file; - } - /** * Return the server-relative url to this item, eg: * /gallery3/index.php/BobsWedding?page=2 -- cgit v1.2.3 From 1b3a6b85c156e4777d2aa8205b130984f55dc66d Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 30 Apr 2011 18:29:34 -0600 Subject: Improve the comment explaining why the data_file extension is important. --- modules/gallery/models/item.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 22634cbf..81830fb9 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -409,7 +409,11 @@ class Item_Model_Core extends ORM_MPTT { // keep it around. $original = ORM::factory("item", $this->id); - // Preserve the extension of the data file. + // Preserve the extension of the data file. Many helpers, (e.g. ImageMagick), assume + // the MIME type from the extension. So when we adopt the new data file, it's important + // to adopt the new extension. That ensures that the item's extension is always + // appropriate for its data. We don't try to preserve the name of the data file, though, + // because the name is typically a temporary randomly-generated name. if (isset($this->data_file)) { $extension = pathinfo($this->data_file, PATHINFO_EXTENSION); $new_name = pathinfo($this->name, PATHINFO_FILENAME) . ".$extension"; -- cgit v1.2.3 From f0f094c3f79b09536f58083681c28f73271c506d Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Sat, 30 Apr 2011 20:22:49 -0600 Subject: Explain the conditional rename in item::save() with a comment. --- modules/gallery/models/item.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 81830fb9..807c88a7 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -443,11 +443,19 @@ class Item_Model_Core extends ORM_MPTT { } if ($original->parent_id != $this->parent_id || $original->name != $this->name) { - // Move all of the items associated data files $this->_build_relative_caches(); + // If there is a data file, then we want to preserve both the old data and the new data. + // (Third-party event handlers would like access to both). The old data file will be + // accessible via the $original item, and the new one via $this item. But in that case, + // we don't want to rename the original as below, because the old data would end up being + // clobbered by the new data file. Also, the rename isn't necessary, because the new item + // data is coming from the data file anyway. So we only perform the rename if there isn't + // a data file. Another way to solve this would be to copy the original file rather than + // conditionally rename it, but a copy would cost far more than the rename. if (!isset($this->data_file)) { @rename($original->file_path(), $this->file_path()); } + // Move all of the items associated data files if ($this->is_album()) { @rename(dirname($original->resize_path()), dirname($this->resize_path())); @rename(dirname($original->thumb_path()), dirname($this->thumb_path())); -- cgit v1.2.3 From 72f3fc46f6c7c9043e730063051ecfd88bf314c8 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 4 May 2011 17:22:15 -0600 Subject: Avoid "self::" because Kohana can't override it. --- modules/gallery/helpers/legal_file.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index 2cb0fb25..68403fa6 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -31,7 +31,7 @@ class legal_file_Core { static function get_filters() { $filters = array(); - foreach (self::get_extensions() as $extension) { + foreach (legal_file::get_extensions() as $extension) { array_push($filters, "*." . $extension, "*." . strtoupper($extension)); } return $filters; -- cgit v1.2.3 From 5e62d327a8dc477d3edea99826183548aca3e7f3 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 18 May 2011 20:17:36 -0600 Subject: Expand the legal_file events to include separate photo and movie events, and to support MIME types. --- modules/gallery/helpers/legal_file.php | 36 ++++++++++++++++++++++++++++++---- modules/gallery/models/item.php | 15 ++++++++------ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index 68403fa6..5d10dffd 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -18,15 +18,28 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class legal_file_Core { - static function get_extensions() { + static function get_photo_extensions() { // Create a default list of allowed extensions and then let modules modify it. $extensions_wrapper = new stdClass(); $extensions_wrapper->extensions = array("gif", "jpg", "jpeg", "png"); + module::event("legal_photo_extensions", $extensions_wrapper); + return $extensions_wrapper->extensions; + } + + static function get_movie_extensions() { + // Create a default list of allowed extensions and then let modules modify it. + $extensions_wrapper = new stdClass(); + $extensions_wrapper->extensions = array("flv", "mp4", "m4v"); + module::event("legal_movie_extensions", $extensions_wrapper); + return $extensions_wrapper->extensions; + } + + static function get_extensions() { + $extensions = legal_file::get_photo_extensions(); if (movie::find_ffmpeg()) { - array_push($extensions_wrapper->extensions, "flv", "mp4", "m4v"); + array_push($extensions, legal_file::get_movie_extensions()); } - module::event("legal_file_extensions", $extensions_wrapper); - return $extensions_wrapper->extensions; + return $extensions; } static function get_filters() { @@ -36,4 +49,19 @@ class legal_file_Core { } return $filters; } + + static function get_photo_types() { + // Create a default list of allowed types and then let modules modify it. + $types_wrapper = new stdClass(); + module::event("legal_photo_types", $types_wrapper); + $types_wrapper->types = array("image/jpeg", "image/gif", "image/png"); + } + + static function get_movie_types() { + // Create a default list of allowed types and then let modules modify it. + $types_wrapper = new stdClass(); + $types_wrapper->types = array("video/flv", "video/x-flv", "video/mp4"); + module::event("legal_movie_types", $types_wrapper); + return $types_wrapper->types; + } } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 1704ff6e..1dd9b00b 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -804,10 +804,13 @@ class Item_Model_Core extends ORM_MPTT { return; } - if (($this->is_movie() || $this->is_photo()) && + if ($this->is_photo() && !preg_match("/^(" . - implode("|", array_map("preg_quote", - legal_file::get_extensions())) . + implode("|", array_map("preg_quote", legal_file::get_photo_extensions())) . + ")\$/i", $ext) || + $this->is_movie() && + !preg_match("/^(" . + implode("|", array_map("preg_quote", legal_file::get_movie_extensions())) . ")\$/i", $ext)) { $v->add_error("name", "illegal_data_file_extension"); } @@ -887,9 +890,9 @@ class Item_Model_Core extends ORM_MPTT { switch($field) { case "mime_type": if ($this->is_movie()) { - $legal_values = array("video/flv", "video/x-flv", "video/mp4"); - } if ($this->is_photo()) { - $legal_values = array("image/jpeg", "image/gif", "image/png"); + $legal_values = legal_file::get_movie_types(); + } else if ($this->is_photo()) { + $legal_values = legal_file::get_photo_types(); } break; -- cgit v1.2.3 From e9862d8fbc4d6fd06abf157f48dce671a7283993 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 18 May 2011 20:20:19 -0600 Subject: Correction for the merge conflict markers I accidentally committed. --- modules/gallery/helpers/system.php | 17 ---------------- modules/gallery/tests/System_Helper_Test.php | 30 ---------------------------- 2 files changed, 47 deletions(-) diff --git a/modules/gallery/helpers/system.php b/modules/gallery/helpers/system.php index d2e9c125..4110b4ed 100644 --- a/modules/gallery/helpers/system.php +++ b/modules/gallery/helpers/system.php @@ -47,22 +47,6 @@ class system_Core { * It allows the caller to specify a prefix and an extension. * It always places the file in TMPPATH. */ -<<<<<<< HEAD - static function temp_filename($prefix = "", $extension = "") { - return self::_tempnam(TMPPATH, $prefix, ".$extension", "tempnam"); - } - - /** - * This helper provides a dependency-injected implementation of tempnam. - */ - static function _tempnam($dir, $prefix, $postfix, $builtin) { - do { - $basename = call_user_func($builtin, $dir, $prefix); - if (!$basename) { - return false; - } - $filename = $basename . $postfix; -======= static function temp_filename($prefix="", $extension="") { do { $basename = tempnam(TMPPATH, $prefix); @@ -70,7 +54,6 @@ class system_Core { return false; } $filename = "$basename.$extension"; ->>>>>>> db734130c5fe10408040b2326b28b102f3131271 $success = !file_exists($filename) && @rename($basename, $filename); if (!$success) { @unlink($basename); diff --git a/modules/gallery/tests/System_Helper_Test.php b/modules/gallery/tests/System_Helper_Test.php index 4b395799..3d56c516 100644 --- a/modules/gallery/tests/System_Helper_Test.php +++ b/modules/gallery/tests/System_Helper_Test.php @@ -18,40 +18,10 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class System_Helper_Test extends Gallery_Unit_Test_Case { -<<<<<<< HEAD - public function temp_filename_random_test() { -======= public function temp_filename_test() { ->>>>>>> db734130c5fe10408040b2326b28b102f3131271 $filename = system::temp_filename("file", "ext"); $this->assert_true(file_exists($filename), "File not created"); unlink($filename); $this->assert_pattern($filename, "|/file.*\\.ext$|"); } -<<<<<<< HEAD - - public function tempnam_collision_test() { - require_once('Mock_Built_In.php'); - $existing = TMPPATH . "/file1.ext"; - $available = TMPPATH . "/file2.ext"; - touch($existing); - $filename = system::_tempnam(TMPPATH, "file", ".ext", - array(new Mock_Built_In("1", "2"), "_tempnam")); - unlink($existing); - $this->assert_true(file_exists($filename), "File not created"); - unlink($filename); - $this->assert_equal($available, $filename, "Incorrect filename created"); - } - - public function tempnam_abort_test() { - require_once('Mock_Built_In.php'); - $filename = system::_tempnam(TMPPATH, "file", ".ext", - array(new Mock_Built_In(), "_tempnam")); - if ($filename) { - @unlink($filename); - } - $this->assert_false($filename, "Operation not aborted"); - } -======= ->>>>>>> db734130c5fe10408040b2326b28b102f3131271 } -- cgit v1.2.3 From 4e3964527b66d8ccd76fb261d549cd9861a7a780 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 18 May 2011 20:30:28 -0600 Subject: Initialize legal file arrays correctly. --- modules/gallery/helpers/legal_file.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index 5d10dffd..0d3e9728 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -37,7 +37,7 @@ class legal_file_Core { static function get_extensions() { $extensions = legal_file::get_photo_extensions(); if (movie::find_ffmpeg()) { - array_push($extensions, legal_file::get_movie_extensions()); + $extensions = array_merge($extensions, legal_file::get_movie_extensions()); } return $extensions; } @@ -53,8 +53,9 @@ class legal_file_Core { static function get_photo_types() { // Create a default list of allowed types and then let modules modify it. $types_wrapper = new stdClass(); - module::event("legal_photo_types", $types_wrapper); $types_wrapper->types = array("image/jpeg", "image/gif", "image/png"); + module::event("legal_photo_types", $types_wrapper); + return $types_wrapper->types; } static function get_movie_types() { -- cgit v1.2.3 From e06b20738d0e0bdb80bae68b7fec2b3746192f6e Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 18 May 2011 21:10:08 -0600 Subject: Adding an image representing a broken thumbnail. This image was derived from the equivalent Gallery2 icon. It uses the same washed-out gray color scheme as the Gallery3 missing_movie icon. --- modules/gallery/images/missing_photo.png | Bin 0 -> 1570 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 modules/gallery/images/missing_photo.png diff --git a/modules/gallery/images/missing_photo.png b/modules/gallery/images/missing_photo.png new file mode 100644 index 00000000..67786275 Binary files /dev/null and b/modules/gallery/images/missing_photo.png differ -- cgit v1.2.3 From f2336a5aaa0eb797f252388ecd7b93a82f9646fd Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 18 May 2011 21:56:10 -0600 Subject: Behave reasonably if the image cannot be resized. --- modules/gallery/helpers/graphics.php | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php index acb11bfb..3b9769de 100644 --- a/modules/gallery/helpers/graphics.php +++ b/modules/gallery/helpers/graphics.php @@ -170,23 +170,37 @@ class graphics_Core { foreach (self::_get_rules($target) as $rule) { $args = array($working_file, $output_file, unserialize($rule->args), $item); - call_user_func_array($rule->operation, $args); - $working_file = $output_file; + try { + call_user_func_array($rule->operation, $args); + $working_file = $output_file; + } catch (Exception $e) { + // Ignore this filter and move on. + Kohana_Log::add("error", "Caught exception filtering image: {$item->title}\n" . + $e->getMessage() . "\n" . $e->getTraceAsString()); + } } } if (!empty($ops["thumb"])) { + if (file_exists($item->thumb_path())) { + $item->thumb_dirty = 0; + } else { + copy(MODPATH . "gallery/images/missing_photo.png", $item->thumb_path()); + } $dims = getimagesize($item->thumb_path()); $item->thumb_width = $dims[0]; $item->thumb_height = $dims[1]; - $item->thumb_dirty = 0; } if (!empty($ops["resize"])) { + if (file_exists($item->resize_path())) { + $item->resize_dirty = 0; + } else { + copy(MODPATH . "gallery/images/missing_photo.png", $item->resize_path()); + } $dims = getimagesize($item->resize_path()); $item->resize_width = $dims[0]; $item->resize_height = $dims[1]; - $item->resize_dirty = 0; } $item->save(); } catch (Exception $e) { -- cgit v1.2.3 From 3a89dbb359310c100a66604528376f29a4ad04a1 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Wed, 18 May 2011 22:10:38 -0600 Subject: Removing an unused file. --- modules/gallery/tests/Mock_Built_In.php | 39 --------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 modules/gallery/tests/Mock_Built_In.php diff --git a/modules/gallery/tests/Mock_Built_In.php b/modules/gallery/tests/Mock_Built_In.php deleted file mode 100644 index b02e5ecf..00000000 --- a/modules/gallery/tests/Mock_Built_In.php +++ /dev/null @@ -1,39 +0,0 @@ -nonces = func_get_args(); - } - - function _tempnam($dir, $prefix) { - if (empty($this->nonces)) - return false; - $filename = "$dir/$prefix" . array_shift($this->nonces); - if (!touch($filename)) - return false; - return $filename; - } -} -- cgit v1.2.3