From 1b2da1ff70acba4177a7ebea825f802f24801a0c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 6 Aug 2010 10:41:38 -0700 Subject: Add a "weight" column to the module table. This allows us to specify module ordering, which is currently being done in the moduleorder contrib module. By default, the weight will be the same as the id of the row which means that new modules will get added at the end of the list. This is covered in the upgrade case as well. The one gotcha is that we need to make sure that we don't try to sort by the weight column if the gallery module version is < 32, which is something we haven't done before. Fixes ticket #1272. --- modules/gallery/helpers/gallery_installer.php | 13 +++++++++++-- modules/gallery/helpers/module.php | 20 +++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index f5589618..8fc0cf96 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -144,8 +144,10 @@ class gallery_installer { `active` BOOLEAN default 0, `name` varchar(64) default NULL, `version` int(9) default NULL, + `weight` int(9) default NULL, PRIMARY KEY (`id`), - UNIQUE KEY(`name`)) + UNIQUE KEY(`name`), + KEY (`weight`)) DEFAULT CHARSET=utf8;"); $db->query("CREATE TABLE {outgoing_translations} ( @@ -296,7 +298,7 @@ class gallery_installer { module::set_var("gallery", "simultaneous_upload_limit", 5); module::set_var("gallery", "admin_area_timeout", 90 * 60); module::set_var("gallery", "maintenance_mode", 0); - module::set_version("gallery", 31); + module::set_version("gallery", 32); } static function upgrade($version) { @@ -561,6 +563,13 @@ class gallery_installer { module::set_var("gallery", "maintenance_mode", 0); module::set_version("gallery", $version = 31); } + + if ($version == 31) { + db::update("modules") + ->set("weight", "=", "id") + ->execute(); + module::set_version("gallery", $version = 32); + } } static function uninstall() { diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php index 5134c7b3..ca6651f1 100644 --- a/modules/gallery/helpers/module.php +++ b/modules/gallery/helpers/module.php @@ -166,6 +166,16 @@ class module_Core { } else { module::set_version($module_name, 1); } + + // Set the weight of the new module, which controls the order in which the modules are + // loaded. By default, new modules are installed at the end of the priority list. Since the + // id field is monotonically increasing, the easiest way to guarantee that is to set the weight + // the same as the id. We don't know that until we save it for the first time + $module = ORM::factory("module")->where("name", "=", $module_name)->find(); + if ($module->loaded()) { + $module->weight = $module->id; + $module->save(); + } module::load_modules(); // Now the module is installed but inactive, so don't leave it in the active path @@ -314,7 +324,15 @@ class module_Core { self::$modules = array(); self::$active = array(); $kohana_modules = array(); - foreach (ORM::factory("module")->find_all() as $module) { + + // In version 32 we introduced a weight column so we can specify the module order + // If we try to use that blindly, we'll break earlier versions before they can even + // run the upgrader. + $modules = module::get_version("gallery") < 32 ? + ORM::factory("module")->find_all(): + ORM::factory("module")->order_by("weight")->find_all(); + + foreach ($modules as $module) { self::$modules[$module->name] = $module; if (!$module->active) { continue; -- cgit v1.2.3 From 16ae65464cb33b16d77cb214bebb699158d548a7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 7 Aug 2010 10:57:18 -0700 Subject: Oops. Fix the upgrader path to add the weight column to the modules table. --- modules/gallery/helpers/gallery_installer.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 8fc0cf96..7896a7a7 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -565,8 +565,10 @@ class gallery_installer { } if ($version == 31) { + $db->query("ALTER TABLE {modules} ADD COLUMN `weight` int(9) DEFAULT NULL"); + $db->query("ALTER TABLE {modules} ADD KEY (`weight`)"); db::update("modules") - ->set("weight", "=", "id") + ->set("weight", new Database_Expression("`id`")) ->execute(); module::set_version("gallery", $version = 32); } -- cgit v1.2.3 From 779d91cca01567a09b894e9fff4dfa32cb82d3d9 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 7 Aug 2010 12:18:43 -0700 Subject: Add an index for left_ptr, since we use that in ORM_MPTT::parents() which is on every album page. Bump Gallery module version to 33. --- modules/gallery/helpers/gallery_installer.php | 10 ++++++++-- modules/gallery/module.info | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 7896a7a7..21c47ad5 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -114,7 +114,8 @@ class gallery_installer { KEY `parent_id` (`parent_id`), KEY `type` (`type`), KEY `random` (`rand_key`), - KEY `weight` (`weight` DESC)) + KEY `weight` (`weight` DESC), + KEY `left_ptr` (`left_ptr`)) DEFAULT CHARSET=utf8;"); $db->query("CREATE TABLE {logs} ( @@ -298,7 +299,7 @@ class gallery_installer { module::set_var("gallery", "simultaneous_upload_limit", 5); module::set_var("gallery", "admin_area_timeout", 90 * 60); module::set_var("gallery", "maintenance_mode", 0); - module::set_version("gallery", 32); + module::set_version("gallery", 33); } static function upgrade($version) { @@ -572,6 +573,11 @@ class gallery_installer { ->execute(); module::set_version("gallery", $version = 32); } + + if ($version == 32) { + $db->query("ALTER TABLE {items} ADD KEY (`left_ptr`)"); + module::set_version("gallery", $version = 33); + } } static function uninstall() { diff --git a/modules/gallery/module.info b/modules/gallery/module.info index 59db07de..dbecda03 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,3 +1,3 @@ name = "Gallery 3" description = "Gallery core application" -version = 32 +version = 33 -- cgit v1.2.3 From dfb095a26267f8b68b40add03dfe407966c49b92 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 7 Aug 2010 22:18:28 -0700 Subject: Add the ability to replace the source data file in Item_Model::save(). Refactor the rotate code in Quick_Controller to replace the data file, and then have gallery_event::item_updated_data_file() pick up after the change is saved, rebuild the image and handle album covers. This is much more portable than before and it will allow any mechanism (eg: REST) to replace the source image. --- modules/gallery/controllers/quick.php | 22 ++------- modules/gallery/helpers/gallery_event.php | 14 ++++++ modules/gallery/helpers/movie.php | 44 +++++++++++------- modules/gallery/helpers/photo.php | 12 +++++ modules/gallery/models/item.php | 74 ++++++++++++++++++++----------- modules/gallery/tests/Item_Model_Test.php | 31 +++++++++++++ 6 files changed, 137 insertions(+), 60 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index fee601d9..c34209da 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -36,25 +36,11 @@ class Quick_Controller extends Controller { } if ($degrees) { - gallery_graphics::rotate($item->file_path(), $item->file_path(), - array("degrees" => $degrees)); - - list($item->width, $item->height) = getimagesize($item->file_path()); - $item->resize_dirty= 1; - $item->thumb_dirty= 1; + $tmpfile = tempnam(TMPPATH, "rotate"); + gallery_graphics::rotate($item->file_path(), $tmpfile, array("degrees" => $degrees)); + $item->set_data_file($tmpfile); $item->save(); - - graphics::generate($item); - - // @todo: this is an inadequate way to regenerate album cover thumbnails after rotation. - foreach (ORM::factory("item") - ->where("album_cover_item_id", "=", $item->id) - ->find_all() as $target) { - copy($item->thumb_path(), $target->thumb_path()); - $target->thumb_width = $item->thumb_width; - $target->thumb_height = $item->thumb_height; - $target->save(); - } + unlink($tmpfile); } if (Input::instance()->get("page_type") == "collection") { diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index e3fa5e08..e048118b 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -124,6 +124,20 @@ class gallery_event_Core { } } + static function item_updated_data_file($item) { + graphics::generate($item); + + // Update any places where this is the album cover + foreach (ORM::factory("item") + ->where("album_cover_item_id", "=", $item->id) + ->find_all() as $target) { + copy($item->thumb_path(), $target->thumb_path()); + $target->thumb_width = $item->thumb_width; + $target->thumb_height = $item->thumb_height; + $target->save(); + } + } + static function batch_complete() { // Set the album covers for any items that where we probably deleted the album cover during // this batch. The item may have been deleted, so don't count on it being around. Choose the diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index bbb5b66c..4ff29a7b 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -57,23 +57,6 @@ class movie_Core { return $form; } - - static function getmoviesize($filename) { - $ffmpeg = self::find_ffmpeg(); - if (empty($ffmpeg)) { - throw new Exception("@todo MISSING_FFMPEG"); - } - - $cmd = escapeshellcmd($ffmpeg) . " -i " . escapeshellarg($filename) . " 2>&1"; - $result = `$cmd`; - if (preg_match("/Stream.*?Video:.*?(\d+)x(\d+)/", $result, $regs)) { - list ($width, $height) = array($regs[1], $regs[2]); - } else { - list ($width, $height) = array(0, 0); - } - return array($width, $height); - } - static function extract_frame($input_file, $output_file) { $ffmpeg = self::find_ffmpeg(); if (empty($ffmpeg)) { @@ -114,4 +97,31 @@ class movie_Core { } return $ffmpeg_path; } + + + /** + * Return the width, height, mime_type and extension of the given movie file. + */ + static function get_file_metadata($file_path) { + $ffmpeg = self::find_ffmpeg(); + if (empty($ffmpeg)) { + throw new Exception("@todo MISSING_FFMPEG"); + } + + $cmd = escapeshellcmd($ffmpeg) . " -i " . escapeshellarg($file_path) . " 2>&1"; + $result = `$cmd`; + if (preg_match("/Stream.*?Video:.*?(\d+)x(\d+)/", $result, $regs)) { + list ($width, $height) = array($regs[1], $regs[2]); + } else { + list ($width, $height) = array(0, 0); + } + + $pi = pathinfo($file_path); + $extension = isset($pi["extension"]) ? $pi["extension"] : "flv"; // No extension? Assume FLV. + $mime_type = in_array(strtolower($extension), array("mp4", "m4v")) ? + "video/mp4" : "video/x-flv"; + + return array($width, $height, $mime_type, $extension); + } + } diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 73cd60c0..a38b4fb2 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -77,4 +77,16 @@ class photo_Core { } return sprintf($format, $new_width, $new_height); } + + /** + * Return the width, height, mime_type and extension of the given image file. + */ + static function get_file_metadata($file_path) { + $image_info = getimagesize($file_path); + $width = $image_info[0]; + $height = $image_info[1]; + $mime_type = $image_info["mime"]; + $extension = image_type_to_extension($image_info[2], false); + return array($width, $height, $mime_type, $extension); + } } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index c00b7972..5257bbb9 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -316,7 +316,7 @@ class Item_Model extends ORM_MPTT { unset($significant_changes["relative_url_cache"]); unset($significant_changes["relative_path_cache"]); - if (!empty($this->changed) && $significant_changes) { + if ((!empty($this->changed) && $significant_changes) || isset($this->data_file)) { $this->updated = time(); if (!$this->loaded()) { // Create a new item. @@ -341,30 +341,19 @@ class Item_Model extends ORM_MPTT { } // Get the width, height and mime type from our data file for photos and movies. - if ($this->is_movie() || $this->is_photo()) { - $pi = pathinfo($this->data_file); - + if ($this->is_photo() || $this->is_movie()) { if ($this->is_photo()) { - $image_info = getimagesize($this->data_file); - $this->width = $image_info[0]; - $this->height = $image_info[1]; - $this->mime_type = $image_info["mime"]; - - // Force an extension onto the name if necessary - if (empty($pi["extension"])) { - $pi["extension"] = image_type_to_extension($image_info[2], false); - $this->name .= "." . $pi["extension"]; - } - } else { - list ($this->width, $this->height) = movie::getmoviesize($this->data_file); - - // No extension? Assume FLV. - if (empty($pi["extension"])) { - $pi["extension"] = "flv"; - $this->name .= "." . $pi["extension"]; - } + list ($this->width, $this->height, $this->mime_type, $extension) = + photo::get_file_metadata($this->data_file); + } else if ($this->is_movie()) { + list ($this->width, $this->height, $this->mime_type, $extension) = + movie::get_file_metadata($this->data_file); + } - $this->mime_type = in_array(strtolower($pi["extension"]), array("mp4", "m4v")) ? "video/mp4" : "video/x-flv"; + // Force an extension onto the name if necessary + $pi = pathinfo($this->data_file); + if (empty($pi["extension"])) { + $this->name = "{$this->name}.$extension"; } } @@ -479,7 +468,30 @@ class Item_Model extends ORM_MPTT { ->execute(); } + // 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()); + + // Get the width, height and mime type from our data file for photos and movies. + if ($this->is_photo()) { + list ($this->width, $this->height) = photo::get_file_metadata($this->file_path()); + } else if ($this->is_movie()) { + list ($this->width, $this->height) = movie::get_file_metadata($this->file_path()); + } + $this->thumb_dirty = 1; + $this->resize_dirty = 1; + } + module::event("item_updated", $original, $this); + + if ($this->data_file) { + // 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; + module::event("item_updated_data_file", $this); + } } } else if (!empty($this->changed)) { // Insignificant changes only. Don't fire events or do any special checking to try to keep @@ -765,8 +777,9 @@ class Item_Model extends ORM_MPTT { $this->rules["slug"] = array(); } - // Movies and photos must have data files - if (($this->is_photo() || $this->is_movie()) && !$this->loaded()) { + // Movies and photos must have data files. Verify the data file on new items, or if it has + // been replaced. + if (($this->is_photo() || $this->is_movie()) && $this->data_file) { $this->rules["name"]["callbacks"][] = array($this, "valid_data_file"); } } @@ -842,6 +855,17 @@ class Item_Model 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"); + } + } } /** diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php index 907cfe24..bd123098 100644 --- a/modules/gallery/tests/Item_Model_Test.php +++ b/modules/gallery/tests/Item_Model_Test.php @@ -384,4 +384,35 @@ class Item_Model_Test extends Gallery_Unit_Test_Case { $this->assert_same($photo->id, $album->album_cover_item_id); } + + public function replace_data_file_test() { + // Random photo is modules/gallery/tests/test.jpg which is 1024x768 and 6232 bytes. + $photo = test::random_photo(); + $this->assert_equal(1024, $photo->width); + $this->assert_equal(768, $photo->height); + $this->assert_equal(6232, filesize($photo->file_path())); + + // Random photo is gallery/images/imagemagick.jpg is 114x118 and 20337 bytes + $photo->set_data_file(MODPATH . "gallery/images/imagemagick.jpg"); + $photo->save(); + + $this->assert_equal(114, $photo->width); + $this->assert_equal(118, $photo->height); + $this->assert_equal(20337, filesize($photo->file_path())); + } + + public function replacement_data_file_must_be_same_mime_type_test() { + // Random photo is modules/gallery/tests/test.jpg + $photo = test::random_photo(); + $photo->set_data_file(MODPATH . "gallery/images/graphicsmagick.png"); + + try { + $photo->save(); + } catch (ORM_Validation_Exception $e) { + $this->assert_same(array("name" => "cant_change_mime_type"), $e->validation->errors()); + return; // pass + } + $this->assert_true(false, "Shouldn't get here"); + + } } -- cgit v1.2.3 From 20fd9872965a65121c4497fb166eda15b1a9f360 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 7 Aug 2010 22:33:01 -0700 Subject: A new REST resource that allows access to view and modify the actual contents of the file, which enables REST viewers to see the actual data which is useful when the files are privileged. Currently it returns the contents of the file in JSON encoded form, which may not be the best. Multipart/mime might be much better. Fixes ticket #1224. --- modules/gallery/helpers/data_rest.php | 84 +++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 modules/gallery/helpers/data_rest.php (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php new file mode 100644 index 00000000..ca5acb4a --- /dev/null +++ b/modules/gallery/helpers/data_rest.php @@ -0,0 +1,84 @@ +url); + access::required("view", $item); + + $p = $request->params; + switch (isset($p->size) ? $p->size : "full") { + case "thumb": + $entity = array( + "width" => $item->thumb_width, + "height" => $item->thumb_height, + "path" => $item->thumb_path()); + break; + + case "resize": + $entity = array( + "width" => $item->resize_width, + "height" => $item->resize_height, + "path" => $item->resize_path()); + break; + + default: + case "full": + $entity = array( + "width" => $item->width, + "height" => $item->height, + "path" => $item->file_path()); + break; + } + + $entity["size"] = filesize($entity["path"]); + $entity["contents"] = file_get_contents($entity["path"]); + unset($entity["path"]); + + $result = array( + "url" => $request->url, + "entity" => $entity, + "relationships" => rest::relationships("data", $item)); + return $result; + } + + static function put($request) { + $item = rest::resolve($request->url); + access::required("edit", $item); + + if ($item->is_album()) { + throw new Rest_Exception("Bad Request", 400, array("errors" => array("type" => "invalid"))); + } + + $item->set_data_file($request->file); + $item->save(); + } + + static function resolve($id) { + $item = ORM::factory("item", $id); + if (!access::can("view", $item)) { + throw new Kohana_404_Exception(); + } + return $item; + } + + static function url($item) { + return url::abs_site("rest/data/{$item->id}"); + } +} -- cgit v1.2.3 From 4e95ec843a2bef45e044e2aa3a36fcb590d85464 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Aug 2010 01:12:43 -0700 Subject: Allow item_rest::put() to replace the current data file. Remove data_rest::put() altogether; it's no longer necessary. --- modules/gallery/helpers/data_rest.php | 12 ------------ modules/gallery/helpers/item_rest.php | 6 ++++++ 2 files changed, 6 insertions(+), 12 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php index ca5acb4a..e45a4645 100644 --- a/modules/gallery/helpers/data_rest.php +++ b/modules/gallery/helpers/data_rest.php @@ -58,18 +58,6 @@ class data_rest_Core { return $result; } - static function put($request) { - $item = rest::resolve($request->url); - access::required("edit", $item); - - if ($item->is_album()) { - throw new Rest_Exception("Bad Request", 400, array("errors" => array("type" => "invalid"))); - } - - $item->set_data_file($request->file); - $item->save(); - } - static function resolve($id) { $item = ORM::factory("item", $id); if (!access::can("view", $item)) { diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 6869181d..10f9e16a 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -126,6 +126,12 @@ class item_rest_Core { } } } + + // Replace the data file, if required + if (($item->is_photo() || $item->is_movie()) && isset($request->file)) { + $item->set_data_file($request->file); + } + $item->save(); if (isset($request->params->members) && $item->sort_column == "weight") { -- cgit v1.2.3 From b7700d1eec02caa794629adcc0555d7c9f0c1414 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Aug 2010 09:57:13 -0700 Subject: Require the size parameter. Optional params are confusing. And be robust in the face of a missing data file (movies and albums lack resize, albums lack full size, some albums don't have a thumb if they have no contents, etc) --- modules/gallery/helpers/data_rest.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php index e45a4645..48de2a3a 100644 --- a/modules/gallery/helpers/data_rest.php +++ b/modules/gallery/helpers/data_rest.php @@ -23,7 +23,11 @@ class data_rest_Core { access::required("view", $item); $p = $request->params; - switch (isset($p->size) ? $p->size : "full") { + if (!isset($p->size) || !in_array($p->size, array("thumb", "resize", "full"))) { + throw new Rest_Exception("Bad Request", 400, array("errors" => array("size" => "invalid"))); + } + + switch ($p->size) { case "thumb": $entity = array( "width" => $item->thumb_width, @@ -38,7 +42,6 @@ class data_rest_Core { "path" => $item->resize_path()); break; - default: case "full": $entity = array( "width" => $item->width, @@ -47,8 +50,13 @@ class data_rest_Core { break; } - $entity["size"] = filesize($entity["path"]); - $entity["contents"] = file_get_contents($entity["path"]); + if (file_exists($entity["path"]) && is_file($entity["path"])) { + $entity["size"] = filesize($entity["path"]); + $entity["contents"] = file_get_contents($entity["path"]); + } else { + $entity["size"] = null; + $entity["contents"] = null; + } unset($entity["path"]); $result = array( -- cgit v1.2.3 From 9b5e058dd3a7d0d08fa0fad954a1908b9949e934 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Aug 2010 15:01:38 -0700 Subject: We can always send back the header because Kohana buffers output. --- modules/gallery/helpers/json.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/json.php b/modules/gallery/helpers/json.php index a39db27a..a88608aa 100644 --- a/modules/gallery/helpers/json.php +++ b/modules/gallery/helpers/json.php @@ -25,9 +25,7 @@ class json_Core { * @param mixed $message string or object to json encode and print */ static function reply($message) { - if (!headers_sent()) { - header("Content-Type: application/json; charset=" . Kohana::CHARSET); - } + header("Content-Type: application/json; charset=" . Kohana::CHARSET); print json_encode($message); } } \ No newline at end of file -- cgit v1.2.3 From 2dda8e22a7a32db8a5577aad0cff2b47ac0f9c63 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Aug 2010 16:54:31 -0700 Subject: Use the gallery helper date/time functions wherever we format date time for the browser. Fixes ticket #1278. --- modules/comment/views/comments.html.php | 6 +++--- modules/comment/views/user_profile_comments.html.php | 4 ++-- modules/gallery/helpers/gallery.php | 4 ++-- modules/gallery/tests/xss_data.txt | 2 +- modules/info/views/info_block.html.php | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/comment/views/comments.html.php b/modules/comment/views/comments.html.php index da45f57b..b524f5da 100644 --- a/modules/comment/views/comments.html.php +++ b/modules/comment/views/comments.html.php @@ -36,11 +36,11 @@ author()->guest): ?> date("Y-M-d H:i:s", $comment->created), - "name" => html::clean($comment->author_name()))); ?> + array("date" => gallery::date_time($comment->created), + "name" => html::clean($comment->author_name()))); ?> %name said', - array("date" => date("Y-M-d H:i:s", $comment->created), + array("date" => gallery::date_time($comment->created), "url" => user_profile::url($comment->author_id), "name" => html::clean($comment->author_name()))); ?> diff --git a/modules/comment/views/user_profile_comments.html.php b/modules/comment/views/user_profile_comments.html.php index a2a641ba..377b2d95 100644 --- a/modules/comment/views/user_profile_comments.html.php +++ b/modules/comment/views/user_profile_comments.html.php @@ -4,8 +4,8 @@

  • - date("Y-M-d H:i:s", $comment->created), + gallery::date_time($comment->created), "title" => $comment->item()->title)); ?> item()->thumb_img(array(), 50) ?> diff --git a/modules/gallery/helpers/gallery.php b/modules/gallery/helpers/gallery.php index 54d16322..3f83b23d 100644 --- a/modules/gallery/helpers/gallery.php +++ b/modules/gallery/helpers/gallery.php @@ -60,7 +60,7 @@ class gallery_Core { * @return string */ static function date_time($timestamp) { - return date(module::get_var("gallery", "date_time_format", "Y-M-d H:i:s"), $timestamp); + return date(module::get_var("gallery", "date_time_format"), $timestamp); } /** @@ -69,7 +69,7 @@ class gallery_Core { * @return string */ static function date($timestamp) { - return date(module::get_var("gallery", "date_format", "Y-M-d"), $timestamp); + return date(module::get_var("gallery", "date_format"), $timestamp); } /** diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt index f135c522..ef92970b 100644 --- a/modules/gallery/tests/xss_data.txt +++ b/modules/gallery/tests/xss_data.txt @@ -267,7 +267,7 @@ modules/gallery/views/user_profile.html.php 34 DIRTY_ATTR $use modules/gallery/views/user_profile.html.php 43 DIRTY $info->view modules/image_block/views/image_block_block.html.php 3 DIRTY_JS $item->url() modules/image_block/views/image_block_block.html.php 4 DIRTY $item->thumb_img(array("class"=>"g-thumbnail")) -modules/info/views/info_block.html.php 22 DIRTY date("M j, Y H:i:s",$item->captured) +modules/info/views/info_block.html.php 22 DIRTY gallery::date_time($item->captured) modules/info/views/info_block.html.php 29 DIRTY_JS $item->owner->url modules/notification/views/comment_published.html.php 28 DIRTY_JS $comment->item()->abs_url() modules/notification/views/comment_published.html.php 29 DIRTY $comment->item()->abs_url() diff --git a/modules/info/views/info_block.html.php b/modules/info/views/info_block.html.php index ac177ee7..ebe9bd28 100644 --- a/modules/info/views/info_block.html.php +++ b/modules/info/views/info_block.html.php @@ -19,7 +19,7 @@ captured): ?>

  • - captured)?> + captured)?>
    • owner): ?> -- cgit v1.2.3 From 6a8c1f5e855b1c4d768524e6e542218c234df6d3 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Aug 2010 17:19:51 -0700 Subject: whitespace fix. --- modules/gallery/helpers/identity.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/identity.php b/modules/gallery/helpers/identity.php index 5f1664ec..4febc4e2 100644 --- a/modules/gallery/helpers/identity.php +++ b/modules/gallery/helpers/identity.php @@ -66,7 +66,8 @@ class identity_Core { // The installer cannot set a user into the session, so it just sets an id which we should // upconvert into a user. - // @todo set the user name into the session instead of 2 and then use it to get the user object + // @todo set the user name into the session instead of 2 and then use it to get the + // user object if ($user === 2) { auth::login(IdentityProvider::instance()->admin_user()); } -- cgit v1.2.3 From acb1faaa594fc5067c4340e073afca3b83f819d4 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Aug 2010 17:25:31 -0700 Subject: Cache the group ids for a day to trade off performance for security updates. Fixes ticket #1227. --- modules/gallery/helpers/identity.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/identity.php b/modules/gallery/helpers/identity.php index 4febc4e2..5de05948 100644 --- a/modules/gallery/helpers/identity.php +++ b/modules/gallery/helpers/identity.php @@ -72,12 +72,14 @@ class identity_Core { auth::login(IdentityProvider::instance()->admin_user()); } - if (!$session->get("group_ids")) { + // Cache the group ids for a day to trade off performance for security updates. + if (!$session->get("group_ids") || $session->get("group_ids_timeout", 0) < time()) { $ids = array(); foreach ($user->groups() as $group) { $ids[] = $group->id; } $session->set("group_ids", $ids); + $session->set("group_ids_timeout", time() + 86400); } } catch (Exception $e) { // Log it, so we at least have so notification that we swallowed the exception. -- cgit v1.2.3 From 1ad1f9517f91875875f2e062bda7d834827c3430 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Sun, 8 Aug 2010 17:29:22 -0700 Subject: Fix for ticket #1279. In admin themes sanitize the theme name before checking that theme.info exists. --- modules/gallery/controllers/admin_themes.php | 7 ++++--- modules/gallery/helpers/theme.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/controllers/admin_themes.php b/modules/gallery/controllers/admin_themes.php index e59eadaf..a88e1e89 100644 --- a/modules/gallery/controllers/admin_themes.php +++ b/modules/gallery/controllers/admin_themes.php @@ -31,10 +31,11 @@ class Admin_Themes_Controller extends Admin_Controller { private function _get_themes() { $themes = array(); foreach (scandir(THEMEPATH) as $theme_name) { + if ($theme_name[0] == ".") { + continue; + } + $theme_name = preg_replace("/[^a-zA-Z0-9\._-]/", "", $theme_name); if (file_exists(THEMEPATH . "$theme_name/theme.info")) { - if ($theme_name[0] == ".") { - continue; - } $themes[$theme_name] = theme::get_info($theme_name); } diff --git a/modules/gallery/helpers/theme.php b/modules/gallery/helpers/theme.php index 3589a5b7..9df3eaf2 100644 --- a/modules/gallery/helpers/theme.php +++ b/modules/gallery/helpers/theme.php @@ -111,7 +111,7 @@ class theme_Core { } static function get_info($theme_name) { - $theme_name = preg_replace("/[^\w]/", "", $theme_name); + $theme_name = preg_replace("/[^a-zA-Z0-9\._-]/", "", $theme_name); $file = THEMEPATH . "$theme_name/theme.info"; $theme_info = new ArrayObject(parse_ini_file($file), ArrayObject::ARRAY_AS_PROPS); $theme_info->description = t($theme_info->description); -- cgit v1.2.3 From 3c18762fda9a91717b5defc300ace6bda61eb233 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 9 Aug 2010 22:54:57 -0700 Subject: Change the way that this works. Now instead of sending back the image metadata and the data itself JSON encoded, we just send back the raw data with the right Content-Type. This, combined with code in Item_Model::as_restful_array() that swaps in /rest/data urls as appropriate, means that the RESTful payload has consistent urls when permissions are in play. --- modules/gallery/helpers/data_rest.php | 55 +++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 25 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php index 48de2a3a..3cd2f59a 100644 --- a/modules/gallery/helpers/data_rest.php +++ b/modules/gallery/helpers/data_rest.php @@ -17,6 +17,11 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ + +/** + * This resource returns the raw contents of Item_Model data files. It's analogous to the + * file_proxy controller, but it uses the REST authentication model. + */ class data_rest_Core { static function get($request) { $item = rest::resolve($request->url); @@ -29,41 +34,41 @@ class data_rest_Core { switch ($p->size) { case "thumb": - $entity = array( - "width" => $item->thumb_width, - "height" => $item->thumb_height, - "path" => $item->thumb_path()); + $file = $item->thumb_path(); break; case "resize": - $entity = array( - "width" => $item->resize_width, - "height" => $item->resize_height, - "path" => $item->resize_path()); + $file = $item->resize_path(); break; case "full": - $entity = array( - "width" => $item->width, - "height" => $item->height, - "path" => $item->file_path()); + $file = $item->file_path(); break; } - if (file_exists($entity["path"]) && is_file($entity["path"])) { - $entity["size"] = filesize($entity["path"]); - $entity["contents"] = file_get_contents($entity["path"]); + if (!file_exists($file)) { + throw new Kohana_404_Exception(); + } + + // Note: this code is roughly duplicated in data_rest, so if you modify this, please look to + // see if you should make the same change there as well. + // + // We don't have a cache buster in the url, so don't set cache headers here. + // We don't need to save the session for this request + Session::instance()->abort_save(); + + // Dump out the image. If the item is a movie, then its thumbnail will be a JPG. + if ($item->is_movie() && $p->size == "thumb") { + header("Content-Type: image/jpeg"); } else { - $entity["size"] = null; - $entity["contents"] = null; + header("Content-Type: {$item->mime_type}"); } - unset($entity["path"]); + Kohana::close_buffers(false); + readfile($file); - $result = array( - "url" => $request->url, - "entity" => $entity, - "relationships" => rest::relationships("data", $item)); - return $result; + // We must exit here to keep the regular REST framework reply code from adding more bytes on + // at the end or tinkering with headers. + exit; } static function resolve($id) { @@ -74,7 +79,7 @@ class data_rest_Core { return $item; } - static function url($item) { - return url::abs_site("rest/data/{$item->id}"); + static function url($item, $size) { + return url::abs_site("rest/data/{$item->id}?size=$size"); } } -- cgit v1.2.3 From d9f8c8a295b760bb5b858c96f1658d50c321bd01 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 14 Aug 2010 14:56:28 -0700 Subject: Rebuild access_caches rows if they're missing, as necessary. Fixes ticket #1289. --- modules/gallery/helpers/gallery_task.php | 56 +++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 8 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php index bf1355b8..6a1fc28a 100644 --- a/modules/gallery/helpers/gallery_task.php +++ b/modules/gallery/helpers/gallery_task.php @@ -26,7 +26,9 @@ class gallery_task_Core { const FIX_STATE_RUN_DUPE_SLUGS = 5; const FIX_STATE_START_DUPE_NAMES = 6; const FIX_STATE_RUN_DUPE_NAMES = 7; - const FIX_STATE_DONE = 8; + const FIX_STATE_START_MISSING_ACCESS_CACHES = 8; + const FIX_STATE_RUN_MISSING_ACCESS_CACHES = 9; + const FIX_STATE_DONE = 10; static function available_tasks() { $dirty_count = graphics::find_dirty_images_query()->count_records(); @@ -323,15 +325,14 @@ class gallery_task_Core { $total = $task->get("total"); if (empty($total)) { // mptt: 2 operations for every item - // album audit (permissions and bogus album covers): 1 operation for every album - // dupe slugs: 1 operation for each unique conflicted slug $total = 2 * db::build()->count_records("items"); + // album audit (permissions and bogus album covers): 1 operation for every album $total += db::build()->where("type", "=", "album")->count_records("items"); - foreach (self::find_dupe_slugs() as $row) { - $total++; - } - foreach (self::find_dupe_names() as $row) { - $total++; + // one operation for each missing slug, name and access cache + foreach (array("find_dupe_slugs", "find_dupe_names", "find_missing_access_caches") as $func) { + foreach (self::$func() as $row) { + $total++; + } } $task->set("total", $total); @@ -542,6 +543,36 @@ class gallery_task_Core { $completed++; if (empty($stack)) { + $state = self::FIX_STATE_START_MISSING_ACCESS_CACHES; + } + break; + + case self::FIX_STATE_START_MISSING_ACCESS_CACHES: + $stack = array(); + foreach (self::find_missing_access_caches() as $row) { + $stack[] = $row->id; + } + if ($stack) { + $task->set("stack", implode(" ", $stack)); + $state = self::FIX_STATE_RUN_MISSING_ACCESS_CACHES; + } else { + $state = self::FIX_STATE_DONE; + } + break; + + case self::FIX_STATE_RUN_MISSING_ACCESS_CACHES: + $stack = explode(" ", $task->get("stack")); + $id = array_pop($stack); + $access_cache = ORM::factory("access_cache"); + $access_cache->item_id = $id; + $access_cache->save(); + $task->set("stack", implode(" ", $stack)); + $completed++; + if (empty($stack)) { + // The new cache rows are there, but they're incorrectly populated so we have to fix + // them. If this turns out to be too slow, we'll have to refactor + // access::recalculate_permissions to allow us to do it in slices. + access::recalculate_permissions(item::root()); $state = self::FIX_STATE_DONE; } break; @@ -587,4 +618,13 @@ class gallery_task_Core { ->group_by("parent_name") ->execute(); } + + static function find_missing_access_caches() { + return db::build() + ->select("items.id") + ->from("items") + ->join("access_caches", "items.id", "access_caches.item_id", "left") + ->where("access_caches.id", "is", null) + ->execute(); + } } \ No newline at end of file -- cgit v1.2.3 From 50e3230d79b8736f78ebaa4f1c7e6df1c29b3243 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 14 Aug 2010 15:10:07 -0700 Subject: Add a key on access_caches.item_id. Without this, the Fix task query to find missing access_caches is very slow. Bump Gallery module to v34. --- installer/install.sql | 5 +++-- modules/gallery/helpers/gallery_installer.php | 10 ++++++++-- modules/gallery/module.info | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/installer/install.sql b/installer/install.sql index 0df82086..c6314aa7 100644 --- a/installer/install.sql +++ b/installer/install.sql @@ -10,7 +10,8 @@ CREATE TABLE {access_caches} ( `view_full_2` binary(1) NOT NULL DEFAULT '0', `edit_2` binary(1) NOT NULL DEFAULT '0', `add_2` binary(1) NOT NULL DEFAULT '0', - PRIMARY KEY (`id`) + PRIMARY KEY (`id`), + KEY `item_id` (`item_id`) ) AUTO_INCREMENT=2 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; INSERT INTO {access_caches} VALUES (1,1,'1','0','0','1','0','0'); @@ -243,7 +244,7 @@ CREATE TABLE {modules} ( KEY `weight` (`weight`) ) AUTO_INCREMENT=11 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; -INSERT INTO {modules} VALUES (1,1,'gallery',33,1); +INSERT INTO {modules} VALUES (1,1,'gallery',34,1); INSERT INTO {modules} VALUES (2,1,'user',3,2); INSERT INTO {modules} VALUES (3,1,'comment',3,3); INSERT INTO {modules} VALUES (4,1,'organize',1,4); diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php index 21c47ad5..569c5118 100644 --- a/modules/gallery/helpers/gallery_installer.php +++ b/modules/gallery/helpers/gallery_installer.php @@ -23,7 +23,8 @@ class gallery_installer { $db->query("CREATE TABLE {access_caches} ( `id` int(9) NOT NULL auto_increment, `item_id` int(9), - PRIMARY KEY (`id`)) + PRIMARY KEY (`id`), + KEY (`item_id`)) DEFAULT CHARSET=utf8;"); $db->query("CREATE TABLE {access_intents} ( @@ -299,7 +300,7 @@ class gallery_installer { module::set_var("gallery", "simultaneous_upload_limit", 5); module::set_var("gallery", "admin_area_timeout", 90 * 60); module::set_var("gallery", "maintenance_mode", 0); - module::set_version("gallery", 33); + module::set_version("gallery", 34); } static function upgrade($version) { @@ -578,6 +579,11 @@ class gallery_installer { $db->query("ALTER TABLE {items} ADD KEY (`left_ptr`)"); module::set_version("gallery", $version = 33); } + + if ($version == 33) { + $db->query("ALTER TABLE {access_caches} ADD KEY (`item_id`)"); + module::set_version("gallery", $version = 34); + } } static function uninstall() { diff --git a/modules/gallery/module.info b/modules/gallery/module.info index dbecda03..084a0945 100644 --- a/modules/gallery/module.info +++ b/modules/gallery/module.info @@ -1,3 +1,3 @@ name = "Gallery 3" description = "Gallery core application" -version = 33 +version = 34 -- cgit v1.2.3 From b562751fdb2ae8aab3a344e84176ea03381ca04c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 14 Aug 2010 15:45:16 -0700 Subject: Don't expose members of an item that are not viewable by the end user. This leaks item ids, but no other information about the item. Fixes ticket #1292. --- modules/gallery/helpers/items_rest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/items_rest.php b/modules/gallery/helpers/items_rest.php index 9cca9a54..f0b68d63 100644 --- a/modules/gallery/helpers/items_rest.php +++ b/modules/gallery/helpers/items_rest.php @@ -80,7 +80,7 @@ class items_rest_Core { "relationships" => rest::relationships("item", $item)); if ($item->type == "album") { $members = array(); - foreach ($item->children() as $child) { + foreach ($item->viewable()->children() as $child) { $members[] = rest::url("item", $child); } $item_rest["members"] = $members; -- cgit v1.2.3 From 6563ad1393b6d9a9cde44a127355359edae54843 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 14 Aug 2010 22:00:53 -0700 Subject: Return the right content type for album thumbnails (based on the album cover's mime type) --- modules/gallery/helpers/data_rest.php | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php index 3cd2f59a..98c98894 100644 --- a/modules/gallery/helpers/data_rest.php +++ b/modules/gallery/helpers/data_rest.php @@ -57,9 +57,17 @@ class data_rest_Core { // We don't need to save the session for this request Session::instance()->abort_save(); + if ($item->is_album() && !$item->album_cover_item_id) { + // No thumbnail. Return nothing. + // @todo: what should we do here? + return; + } + // Dump out the image. If the item is a movie, then its thumbnail will be a JPG. if ($item->is_movie() && $p->size == "thumb") { header("Content-Type: image/jpeg"); + } else if ($item->is_album()) { + header("Content-Type: " . $item->album_cover()->mime_type); } else { header("Content-Type: {$item->mime_type}"); } -- cgit v1.2.3