From 88350c5b88abb8bfc56b26177b64e049f6b7440f Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 21 Sep 2009 21:21:52 -0700 Subject: Update the next/previous item calculations to match what we do in photos.php Force the children_count to be zero, movies have no children. Rename $photo to $movie everywhere. --- modules/gallery/controllers/movies.php | 86 +++++++++++++++------------------- 1 file changed, 38 insertions(+), 48 deletions(-) (limited to 'modules/gallery/controllers/movies.php') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 04e15315..fa07668e 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -22,42 +22,32 @@ class Movies_Controller extends Items_Controller { /** * @see REST_Controller::_show($resource) */ - public function _show($photo) { - access::required("view", $photo); + public function _show($movie) { + access::required("view", $movie); - // We sort by id ascending so for now, find sibling info by doing id based queries. - $next_item = ORM::factory("item") - ->viewable() - ->where("parent_id", $photo->parent_id) - ->where("id >", $photo->id) - ->orderby("id", "ASC") - ->find(); - $previous_item = ORM::factory("item") - ->viewable() - ->where("parent_id", $photo->parent_id) - ->where("id <", $photo->id) - ->orderby("id", "DESC") - ->find(); - $position = ORM::factory("item") - ->viewable() - ->where("parent_id", $photo->parent_id) - ->where("id <=", $photo->id) - ->count_all(); + $position = $movie->parent()->get_position($movie); + if ($position > 1) { + list ($previous_item, $ignore, $next_item) = + $movie->parent()->children(3, $position - 2); + } else { + $previous_item = null; + list ($next_item) = $movie->parent()->viewable()->children(1, $position); + } $template = new Theme_View("page.html", "movie"); - $template->set_global("item", $photo); + $template->set_global("item", $movie); $template->set_global("children", array()); - $template->set_global("children_count", $photo->children_count()); - $template->set_global("parents", $photo->parents()); - $template->set_global("next_item", $next_item->loaded ? $next_item : null); - $template->set_global("previous_item", $previous_item->loaded ? $previous_item : null); - $template->set_global("sibling_count", $photo->parent()->children_count()); + $template->set_global("children_count", 0); + $template->set_global("parents", $movie->parents()); + $template->set_global("next_item", $next_item); + $template->set_global("previous_item", $previous_item); + $template->set_global("sibling_count", $movie->parent()->viewable()->children_count()); $template->set_global("position", $position); $template->content = new View("movie.html"); - $photo->view_count++; - $photo->save(); + $movie->view_count++; + $movie->save(); print $template; } @@ -65,21 +55,21 @@ class Movies_Controller extends Items_Controller { /** * @see REST_Controller::_update($resource) */ - public function _update($photo) { + public function _update($movie) { access::verify_csrf(); - access::required("view", $photo); - access::required("edit", $photo); + access::required("view", $movie); + access::required("edit", $movie); - $form = photo::get_edit_form($photo); + $form = photo::get_edit_form($movie); if ($valid = $form->validate()) { - if ($form->edit_item->filename->value != $photo->name || - $form->edit_item->slug->value != $photo->slug) { + if ($form->edit_item->filename->value != $movie->name || + $form->edit_item->slug->value != $movie->slug) { // Make sure that there's not a name or slug conflict if ($row = Database::instance() ->select(array("name", "slug")) ->from("items") - ->where("parent_id", $photo->parent_id) - ->where("id <>", $photo->id) + ->where("parent_id", $movie->parent_id) + ->where("id <>", $movie->id) ->open_paren() ->where("name", $form->edit_item->filename->value) ->orwhere("slug", $form->edit_item->slug->value) @@ -98,16 +88,16 @@ class Movies_Controller extends Items_Controller { } if ($valid) { - $photo->title = $form->edit_item->title->value; - $photo->description = $form->edit_item->description->value; - $photo->slug = $form->edit_item->slug->value; - $photo->rename($form->edit_item->filename->value); - $photo->save(); - module::event("item_edit_form_completed", $photo, $form); + $movie->title = $form->edit_item->title->value; + $movie->description = $form->edit_item->description->value; + $movie->slug = $form->edit_item->slug->value; + $movie->rename($form->edit_item->filename->value); + $movie->save(); + module::event("item_edit_form_completed", $movie, $form); - log::success("content", "Updated movie", "url()}\">view"); + log::success("content", "Updated movie", "url()}\">view"); message::success( - t("Saved movie %movie_title", array("movie_title" => $photo->title))); + t("Saved movie %movie_title", array("movie_title" => $movie->title))); print json_encode( array("result" => "success")); @@ -121,9 +111,9 @@ class Movies_Controller extends Items_Controller { /** * @see REST_Controller::_form_edit($resource) */ - public function _form_edit($photo) { - access::required("view", $photo); - access::required("edit", $photo); - print photo::get_edit_form($photo); + public function _form_edit($movie) { + access::required("view", $movie); + access::required("edit", $movie); + print photo::get_edit_form($movie); } } -- cgit v1.2.3 From b79129e3651a92250b61a501b8c6a1d53bf4928c Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 23 Sep 2009 12:02:35 -0700 Subject: Clone the photo::get_edit_form to the movies helper and use it to generate the movie edit form. Fixes ticket #726. --- modules/gallery/controllers/movies.php | 4 ++-- modules/gallery/helpers/album.php | 5 +++-- modules/gallery/helpers/gallery.php | 19 ++++++++++++++++--- modules/gallery/helpers/movie.php | 30 ++++++++++++++++++++++++++++++ modules/gallery/helpers/photo.php | 5 +++-- 5 files changed, 54 insertions(+), 9 deletions(-) (limited to 'modules/gallery/controllers/movies.php') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index fa07668e..2a917c58 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -60,7 +60,7 @@ class Movies_Controller extends Items_Controller { access::required("view", $movie); access::required("edit", $movie); - $form = photo::get_edit_form($movie); + $form = movie::get_edit_form($movie); if ($valid = $form->validate()) { if ($form->edit_item->filename->value != $movie->name || $form->edit_item->slug->value != $movie->slug) { @@ -114,6 +114,6 @@ class Movies_Controller extends Items_Controller { public function _form_edit($movie) { access::required("view", $movie); access::required("edit", $movie); - print photo::get_edit_form($movie); + print movie::get_edit_form($movie); } } diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php index 9cd746d7..3c62e41e 100644 --- a/modules/gallery/helpers/album.php +++ b/modules/gallery/helpers/album.php @@ -123,14 +123,15 @@ class album_Core { if ($parent->id != 1) { $group->input("dirname")->label(t("Directory Name"))->value($parent->name) ->rules("required") - ->error_messages("name_conflict", t("There is already a photo or album with this name")) + ->error_messages( + "name_conflict", t("There is already amovie, photo or album with this name")) ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The directory name can't contain a \"/\"")) ->callback("item::validate_no_trailing_period") ->error_messages("no_trailing_period", t("The directory name can't end in \".\"")); $group->input("slug")->label(t("Internet Address"))->value($parent->slug) ->error_messages( - "slug_conflict", t("There is already a photo or album with this internet address")) + "slug_conflict", t("There is already a movie, photo or album with this internet address")) ->callback("item::validate_url_safe") ->error_messages( "not_url_safe", diff --git a/modules/gallery/helpers/gallery.php b/modules/gallery/helpers/gallery.php index 80ae65bd..91dd2073 100644 --- a/modules/gallery/helpers/gallery.php +++ b/modules/gallery/helpers/gallery.php @@ -114,19 +114,32 @@ class gallery_Core { } } + switch ($item->type) { + case "album": + $option_text = t("Album options"); + $edit_text = t("Edit album"); + break; + case "movie": + $option_text = t("Movie options"); + $edit_text = t("Edit movie"); + break; + default: + $option_text = t("Photo options"); + $edit_text = t("Edit photo"); + } + $menu->append($options_menu = Menu::factory("submenu") ->id("options_menu") - ->label(t("Photo options"))); + ->label($option_text)); if ($item && ($can_edit || $can_add)) { if ($can_edit) { $options_menu->append(Menu::factory("dialog") ->id("edit_item") - ->label($item->is_album() ? t("Edit album") : t("Edit photo")) + ->label($edit_text) ->url(url::site("form/edit/{$item->type}s/$item->id"))); } if ($item->is_album()) { - $options_menu->label(t("Album options")); if ($can_edit) { $options_menu->append(Menu::factory("dialog") ->id("edit_permissions") diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 59bf5c19..6c8c6c88 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -128,6 +128,36 @@ class movie_Core { return $movie; } + static function get_edit_form($movie) { + $form = new Forge("movies/$movie->id", "", "post", array("id" => "gEditMovieForm")); + $form->hidden("_method")->value("put"); + $group = $form->group("edit_item")->label(t("Edit Movie")); + $group->input("title")->label(t("Title"))->value($movie->title); + $group->textarea("description")->label(t("Description"))->value($movie->description); + $group->input("filename")->label(t("Filename"))->value($movie->name) + ->error_messages( + "name_conflict", t("There is already a movie, photo or album with this name")) + ->callback("item::validate_no_slashes") + ->error_messages("no_slashes", t("The movie name can't contain a \"/\"")) + ->callback("item::validate_no_trailing_period") + ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")); + $group->input("slug")->label(t("Internet Address"))->value($movie->slug) + ->callback("item::validate_url_safe") + ->error_messages( + "slug_conflict", t("There is already a movie, photo or album with this internet address")) + ->error_messages( + "not_url_safe", + t("The internet address should contain only letters, numbers, hyphens and underscores")); + + module::event("item_edit_form", $movie, $form); + + $group = $form->group("buttons")->label(""); + $group->submit("")->value(t("Modify")); + $form->add_rules_from(ORM::factory("item")); + return $form; + } + + static function getmoviesize($filename) { $ffmpeg = self::find_ffmpeg(); if (empty($ffmpeg)) { diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 3d9fbe69..065d2d31 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -163,7 +163,8 @@ class photo_Core { $group->input("title")->label(t("Title"))->value($photo->title); $group->textarea("description")->label(t("Description"))->value($photo->description); $group->input("filename")->label(t("Filename"))->value($photo->name) - ->error_messages("name_conflict", t("There is already a photo or album with this name")) + ->error_messages( + "name_conflict", t("There is already a movie, photo or album with this name")) ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The photo name can't contain a \"/\"")) ->callback("item::validate_no_trailing_period") @@ -171,7 +172,7 @@ class photo_Core { $group->input("slug")->label(t("Internet Address"))->value($photo->slug) ->callback("item::validate_url_safe") ->error_messages( - "slug_conflict", t("There is already a photo or album with this internet address")) + "slug_conflict", t("There is already a movie, photo or album with this internet address")) ->error_messages( "not_url_safe", t("The internet address should contain only letters, numbers, hyphens and underscores")); -- cgit v1.2.3 From 0a66ef9cc785fa5fb3614e7664c424d13ff09728 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 13 Oct 2009 10:36:50 -0700 Subject: Don't allow users to change the file extension of photos/movies If you can change the extension, then you can alter the way the server handles the file, which is a security problem. So for example, you can change a .JPG to a .PHP and then if you put some malicious PHP code in the EXIF data, you can get the server to execute it. Vulnerability is low because only users who have edit permissions could do this. Fixes ticket #846 --- modules/gallery/controllers/movies.php | 13 ++++++++++++- modules/gallery/controllers/photos.php | 12 +++++++++++- modules/gallery/helpers/movie.php | 3 ++- modules/gallery/helpers/photo.php | 3 ++- 4 files changed, 27 insertions(+), 4 deletions(-) (limited to 'modules/gallery/controllers/movies.php') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 2a917c58..01a9fc8b 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -61,7 +61,18 @@ class Movies_Controller extends Items_Controller { access::required("edit", $movie); $form = movie::get_edit_form($movie); - if ($valid = $form->validate()) { + $valid = $form->validate(); + + if ($valid) { + $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); + $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION); + if (strcasecmp($new_ext, $old_ext)) { + $form->edit_item->filename->add_error("illegal_extension", 1); + $valid = false; + } + } + + if ($valid) { if ($form->edit_item->filename->value != $movie->name || $form->edit_item->slug->value != $movie->slug) { // Make sure that there's not a name or slug conflict diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 81e7519e..fbff53ce 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -63,7 +63,17 @@ class Photos_Controller extends Items_Controller { $form = photo::get_edit_form($photo); $valid = $form->validate(); - if ($valid = $form->validate()) { + + if ($valid) { + $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); + $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION); + if (strcasecmp($new_ext, $old_ext)) { + $form->edit_item->filename->add_error("illegal_extension", 1); + $valid = false; + } + } + + if ($valid) { if ($form->edit_item->filename->value != $photo->name || $form->edit_item->slug->value != $photo->slug) { // Make sure that there's not a name or slug conflict diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 98419387..9ca28fe6 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -141,7 +141,8 @@ class movie_Core { ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The movie name can't contain a \"/\"")) ->callback("item::validate_no_trailing_period") - ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")); + ->error_messages("no_trailing_period", t("The movie name can't end in \".\"")) + ->error_messages("illegal_extension", t("You cannot change the filename extension")); $group->input("slug")->label(t("Internet Address"))->value($movie->slug) ->callback("item::validate_url_safe") ->error_messages( diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index 99f28753..6677ddc9 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -169,7 +169,8 @@ class photo_Core { ->callback("item::validate_no_slashes") ->error_messages("no_slashes", t("The photo name can't contain a \"/\"")) ->callback("item::validate_no_trailing_period") - ->error_messages("no_trailing_period", t("The photo name can't end in \".\"")); + ->error_messages("no_trailing_period", t("The photo name can't end in \".\"")) + ->error_messages("illegal_extension", t("You cannot change the filename extension")); $group->input("slug")->label(t("Internet Address"))->value($photo->slug) ->callback("item::validate_url_safe") ->error_messages( -- cgit v1.2.3 From 88852c45eac5981a4cd82f3207df1fad0cf51fcc Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 4 Nov 2009 09:50:49 -0800 Subject: Modified the so that a where clause can be passed into item::get_position. Was also able to remove the sub-select from the calculation of the current position as we already have the child item containing the sort column value. Also added a where clause that ignores albums to the get_position, children and children_count method calls in photos.php and movies.php --- modules/gallery/controllers/movies.php | 9 +++++---- modules/gallery/controllers/photos.php | 9 +++++---- modules/gallery/models/item.php | 34 ++++++++++++++++++---------------- 3 files changed, 28 insertions(+), 24 deletions(-) (limited to 'modules/gallery/controllers/movies.php') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 01a9fc8b..6200e8b4 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -25,13 +25,14 @@ class Movies_Controller extends Items_Controller { public function _show($movie) { access::required("view", $movie); - $position = $movie->parent()->get_position($movie); + $where = array("type != " => "album"); + $position = $movie->parent()->get_position($movie, $where); if ($position > 1) { list ($previous_item, $ignore, $next_item) = - $movie->parent()->children(3, $position - 2); + $movie->parent()->children(3, $position - 2, $where); } else { $previous_item = null; - list ($next_item) = $movie->parent()->viewable()->children(1, $position); + list ($next_item) = $movie->parent()->viewable()->children(1, $position, $where); } $template = new Theme_View("page.html", "movie"); @@ -41,7 +42,7 @@ class Movies_Controller extends Items_Controller { $template->set_global("parents", $movie->parents()); $template->set_global("next_item", $next_item); $template->set_global("previous_item", $previous_item); - $template->set_global("sibling_count", $movie->parent()->viewable()->children_count()); + $template->set_global("sibling_count", $movie->parent()->viewable()->children_count($where)); $template->set_global("position", $position); $template->content = new View("movie.html"); diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 54cd63c6..b9adfd90 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -25,13 +25,14 @@ class Photos_Controller extends Items_Controller { public function _show($photo) { access::required("view", $photo); - $position = $photo->parent()->get_position($photo); + $where = array("type != " => "album"); + $position = $photo->parent()->get_position($photo, $where); if ($position > 1) { list ($previous_item, $ignore, $next_item) = - $photo->parent()->children(3, $position - 2); + $photo->parent()->children(3, $position - 2, $where); } else { $previous_item = null; - list ($next_item) = $photo->parent()->viewable()->children(1, $position); + list ($next_item) = $photo->parent()->viewable()->children(1, $position, $where); } $template = new Theme_View("page.html", "photo"); @@ -41,7 +42,7 @@ class Photos_Controller extends Items_Controller { $template->set_global("parents", $photo->parents()); $template->set_global("next_item", $next_item); $template->set_global("previous_item", $previous_item); - $template->set_global("sibling_count", $photo->parent()->viewable()->children_count()); + $template->set_global("sibling_count", $photo->parent()->viewable()->children_count($where)); $template->set_global("position", $position); $template->content = new View("photo.html"); diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index fc0f0193..9735ed62 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -422,7 +422,7 @@ class Item_Model extends ORM_MPTT { * Find the position of the given child id in this album. The resulting value is 1-indexed, so * the first child in the album is at position 1. */ - public function get_position($child) { + public function get_position($child, $where=array()) { if ($this->sort_order == "DESC") { $comp = ">"; } else { @@ -435,18 +435,20 @@ class Item_Model extends ORM_MPTT { $count = $db->from("items") ->where("parent_id", $this->id) ->where($this->sort_column, NULL) + ->where($where) ->count_records(); if (empty($count)) { // There are no NULLs in the sort column, so we can just use it directly. - $position = $db->query(" - SELECT COUNT(*) AS position FROM {items} - WHERE `parent_id` = {$this->id} - AND `{$this->sort_column}` $comp (SELECT `{$this->sort_column}` - FROM {items} WHERE `id` = $child->id)") - ->current()->position; - - // We stopped short of our target value in the sort (notice that we're using a < comparator + $sort_column = $this->sort_column; + + $position = $db->from("items") + ->where("parent_id", $this->id) + ->where("$sort_column < ", $child->$sort_column) + ->where($where) + ->count_records(); + + // We stopped short of our target value in the sort (notice that we're using a < comparator // above) because it's possible that we have duplicate values in the sort column. An // equality check would just arbitrarily pick one of those multiple possible equivalent // columns, which would mean that if you choose a sort order that has duplicates, it'd pick @@ -454,13 +456,12 @@ class Item_Model extends ORM_MPTT { // // Fix this by doing a 2nd query where we iterate over the equivalent columns and add them to // our base value. - $result = $db->query(" - SELECT id FROM {items} - WHERE `parent_id` = {$this->id} - AND `{$this->sort_column}` = (SELECT `{$this->sort_column}` - FROM {items} WHERE `id` = $child->id) - ORDER BY `id` ASC"); - foreach ($result as $row) { + foreach ($db->from("items") + ->where("parent_id", $this->id) + ->where($sort_column, $child->$sort_column) + ->where($where) + ->orderby(array("id" => "ASC")) + ->get() as $row) { $position++; if ($row->id == $child->id) { break; @@ -484,6 +485,7 @@ class Item_Model extends ORM_MPTT { foreach ($db->select("id") ->from("items") ->where("parent_id", $this->id) + ->where($where) ->orderby($orderby) ->get() as $row) { $position++; -- cgit v1.2.3 From d349400531c780715a54ff9537406546217c26ec Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 8 Nov 2009 14:02:09 -0800 Subject: Fix an accidental old use of $photo in the extension renaming code. --- modules/gallery/controllers/movies.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/controllers/movies.php') diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 6200e8b4..5e78376b 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -66,7 +66,7 @@ class Movies_Controller extends Items_Controller { if ($valid) { $new_ext = pathinfo($form->edit_item->filename->value, PATHINFO_EXTENSION); - $old_ext = pathinfo($photo->name, PATHINFO_EXTENSION); + $old_ext = pathinfo($movie->name, PATHINFO_EXTENSION); if (strcasecmp($new_ext, $old_ext)) { $form->edit_item->filename->add_error("illegal_extension", 1); $valid = false; -- cgit v1.2.3