From a6581ede0b7a50c6159eb5d36cf6be340a072609 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 21 Sep 2009 11:35:27 -0700 Subject: Fix Item_Model::get_position() so that our sort is stable when the comparison row has a null value in the sort field. Fix for #627 Note: this changes get_position() to take an Item_Model instead of an id! --- modules/gallery/controllers/photos.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/controllers/photos.php') diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 79ad674a..e6154535 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -25,7 +25,7 @@ class Photos_Controller extends Items_Controller { public function _show($photo) { access::required("view", $photo); - $position = $photo->parent()->get_position($photo->id); + $position = $photo->parent()->get_position($photo); if ($position > 1) { list ($previous_item, $ignore, $next_item) = $photo->parent()->children(3, $position - 2); -- cgit v1.2.3 From 9e6be40e31b06e5dffe7552928cb8b2d9ee7ad59 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 21 Sep 2009 20:47:55 -0700 Subject: Add viewable() protection to children() and children_count() calls. This is not currently necessary (nor is it a security hole) because we don't constrain permissions at the child level in the core, but it makes our security audits easier and will enable the scenario where somebody writes a module to add per-photo permissions. --- modules/gallery/controllers/photos.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/gallery/controllers/photos.php') diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index e6154535..3de9b3ee 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -31,7 +31,7 @@ class Photos_Controller extends Items_Controller { $photo->parent()->children(3, $position - 2); } else { $previous_item = null; - list ($next_item) = $photo->parent()->children(1, $position); + list ($next_item) = $photo->parent()->viewable()->children(1, $position); } $template = new Theme_View("page.html", "photo"); @@ -41,7 +41,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()->children_count()); + $template->set_global("sibling_count", $photo->parent()->viewable()->children_count()); $template->set_global("position", $position); $template->content = new View("photo.html"); -- cgit v1.2.3 From 123afc954281c1f924f851a33ae5016774e6d9f3 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 21 Sep 2009 21:22:07 -0700 Subject: Set children_count to 0, photos have no children. --- modules/gallery/controllers/photos.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery/controllers/photos.php') diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 3de9b3ee..81e7519e 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -37,7 +37,7 @@ class Photos_Controller extends Items_Controller { $template = new Theme_View("page.html", "photo"); $template->set_global("item", $photo); $template->set_global("children", array()); - $template->set_global("children_count", $photo->children_count()); + $template->set_global("children_count", 0); $template->set_global("parents", $photo->parents()); $template->set_global("next_item", $next_item); $template->set_global("previous_item", $previous_item); -- 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/photos.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 95f3eb3aad3700c883ef6d865ae8d6512883b432 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 22 Oct 2009 07:37:14 -0700 Subject: When an album or photo is updated always return the photo/album location as part of the response. This insures that if the internet address changes, then the page will reload properly. --- modules/gallery/controllers/albums.php | 3 ++- modules/gallery/controllers/photos.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'modules/gallery/controllers/photos.php') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 9733d1cd..ae4ad395 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -216,7 +216,8 @@ class Albums_Controller extends Items_Controller { array("album_title" => html::purify($album->title)))); print json_encode( - array("result" => "success")); + array("result" => "success", + "location" => $album->url())); } else { print json_encode( array("result" => "error", diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index fbff53ce..6bb5af89 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -113,7 +113,8 @@ class Photos_Controller extends Items_Controller { array("photo_title" => html::purify($photo->title)))); print json_encode( - array("result" => "success")); + array("result" => "success", + "location" => $photo->url())); } else { print json_encode( array("result" => "error", -- cgit v1.2.3 From 4cb9ec1d6d37b49ebafc68d0a94d794a1acb8b28 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Thu, 22 Oct 2009 10:09:25 -0700 Subject: Use the request::referrer to determine if we are editting the photo or album from the context menu or from its photo or album page. Fixes ticket #745. Thanks to jankoprowski for the referrer approach. --- modules/gallery/controllers/albums.php | 4 +++- modules/gallery/controllers/photos.php | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'modules/gallery/controllers/photos.php') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 9480a037..fabf67ce 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -200,6 +200,8 @@ class Albums_Controller extends Items_Controller { } if ($valid) { + $watching_album = $album->url() != ($location = parse_url(request::referrer(), PHP_URL_PATH)); + $album->title = $form->edit_item->title->value; $album->description = $form->edit_item->description->value; $album->sort_column = $form->edit_item->sort_order->column->value; @@ -217,7 +219,7 @@ class Albums_Controller extends Items_Controller { print json_encode( array("result" => "success", - "location" => $album->url())); + "location" => $watching_album ? $location : $album->url())); } else { print json_encode( array("result" => "error", diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 6bb5af89..54cd63c6 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -100,6 +100,8 @@ class Photos_Controller extends Items_Controller { } if ($valid) { + $watching_album = $photo->url() != ($location = parse_url(request::referrer(), PHP_URL_PATH)); + $photo->title = $form->edit_item->title->value; $photo->description = $form->edit_item->description->value; $photo->slug = $form->edit_item->slug->value; @@ -114,7 +116,7 @@ class Photos_Controller extends Items_Controller { print json_encode( array("result" => "success", - "location" => $photo->url())); + "location" => $watching_album ? $location : $photo->url())); } else { print json_encode( array("result" => "error", -- 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/photos.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