From 592eff0e5a8af6f74eff4806dc6e7de56ca02761 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Sat, 19 Jan 2013 08:40:19 +0100 Subject: #1942 - Make data_rest and file_proxy more consistent - several minor documentation/formatting changes. No actual functionality changed here. --- modules/gallery/controllers/file_proxy.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php index b9ff7df1..9af58c0c 100644 --- a/modules/gallery/controllers/file_proxy.php +++ b/modules/gallery/controllers/file_proxy.php @@ -100,6 +100,9 @@ class File_Proxy_Controller extends Controller { 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. + if ($type == "albums") { $file = $item->file_path(); } else if ($type == "resizes") { -- cgit v1.2.3 From 1927dd00e42062a98855d121ec8427b25d74d015 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Sun, 20 Jan 2013 08:34:12 +0100 Subject: #1949 - Fix album thumb mime types given by data_rest and file_proxy Correct result: always "image/jpeg" Old data_rest result: mime of cover item Old file_proxy result: mime of album item (null) --- modules/gallery/controllers/file_proxy.php | 4 ++-- modules/gallery/helpers/data_rest.php | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php index 9af58c0c..b2120455 100644 --- a/modules/gallery/controllers/file_proxy.php +++ b/modules/gallery/controllers/file_proxy.php @@ -126,8 +126,8 @@ class File_Proxy_Controller extends Controller { expires::set(2592000, $item->updated); // 30 days - // Dump out the image. If the item is a movie, then its thumbnail will be a JPG. - if ($item->is_movie() && $type != "albums") { + // Dump out the image. If the item is a movie or album, then its thumbnail will be a JPG. + if (($item->is_movie() || $item->is_album()) && $type == "thumbs") { header("Content-Type: image/jpeg"); } else { header("Content-Type: $item->mime_type"); diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php index abd4638e..ef4f17e7 100644 --- a/modules/gallery/helpers/data_rest.php +++ b/modules/gallery/helpers/data_rest.php @@ -57,13 +57,11 @@ class data_rest_Core { 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") { + // Dump out the image. If the item is a movie or album, then its thumbnail will be a JPG. + if (($item->is_movie() || $item->is_album()) && $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}"); + header("Content-Type: $item->mime_type"); } Kohana::close_buffers(false); -- cgit v1.2.3 From f1d2a8e871327d250574d2dd7cacbb21ea3ae995 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 20 Jan 2013 23:54:01 -0500 Subject: Add a long overdue test for File_Proxy_Controller that tests all the various edge case behaviors. It doesn't cover the various headers, but it does cover the permission based code paths. --- modules/gallery/controllers/file_proxy.php | 34 ++++-- .../gallery/tests/File_Proxy_Controller_Test.php | 130 +++++++++++++++++++++ 2 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 modules/gallery/tests/File_Proxy_Controller_Test.php (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php index b2120455..df1f7908 100644 --- a/modules/gallery/controllers/file_proxy.php +++ b/modules/gallery/controllers/file_proxy.php @@ -49,7 +49,9 @@ class File_Proxy_Controller extends Controller { // Make sure that the request is for a file inside var $offset = strpos(rawurldecode($request_uri), $var_uri); if ($offset !== 0) { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 1; + throw $e; } // file_uri: albums/foo/bar.jpg @@ -59,7 +61,9 @@ class File_Proxy_Controller extends Controller { // path: foo/bar.jpg list ($type, $path) = explode("/", $file_uri, 2); if ($type != "resizes" && $type != "albums" && $type != "thumbs") { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 2; + throw $e; } // If the last element is .album.jpg, pop that off since it's not a real item @@ -82,22 +86,30 @@ class File_Proxy_Controller extends Controller { } if (!$item->loaded()) { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 3; + throw $e; } // Make sure we have access to the item if (!access::can("view", $item)) { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 4; + throw $e; } // Make sure we have view_full access to the original if ($type == "albums" && !access::can("view_full", $item)) { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 5; + throw $e; } // Don't try to load a directory if ($type == "albums" && $item->is_album()) { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 6; + throw $e; } // Note: this code is roughly duplicated in data_rest, so if you modify this, please look to @@ -112,7 +124,9 @@ class File_Proxy_Controller extends Controller { } if (!file_exists($file)) { - throw new Kohana_404_Exception(); + $e = new Kohana_404_Exception(); + $e->test_fail_code = 7; + throw $e; } header("Content-Length: " . filesize($file)); @@ -146,6 +160,10 @@ class File_Proxy_Controller extends Controller { } } - readfile($file); + if (TEST_MODE) { + return $file; + } else { + readfile($file); + } } } diff --git a/modules/gallery/tests/File_Proxy_Controller_Test.php b/modules/gallery/tests/File_Proxy_Controller_Test.php new file mode 100644 index 00000000..dab2b8f3 --- /dev/null +++ b/modules/gallery/tests/File_Proxy_Controller_Test.php @@ -0,0 +1,130 @@ +_save = array($_SERVER); + } + + public function teardown() { + list($_SERVER) = $this->_save; + identity::set_active_user(identity::admin_user()); + } + + public function basic_test() { + $photo = test::random_photo(); + $_SERVER["REQUEST_URI"] = url::file("var/albums/{$photo->name}"); + $controller = new File_Proxy_Controller(); + $this->assert_same($photo->file_path(), $controller->__call("", array())); + } + + public function query_params_are_ignored_test() { + $photo = test::random_photo(); + $_SERVER["REQUEST_URI"] = url::file("var/albums/{$photo->name}?a=1&b=2"); + $controller = new File_Proxy_Controller(); + $this->assert_same($photo->file_path(), $controller->__call("", array())); + } + + public function file_must_be_in_var_test() { + $_SERVER["REQUEST_URI"] = url::file("index.php"); + $controller = new File_Proxy_Controller(); + try { + $controller->__call("", array()); + $this->assert_true(false); + } catch (Kohana_404_Exception $e) { + $this->assert_same(1, $e->test_fail_code); + } + } + + public function file_must_be_in_albums_thumbs_or_resizes_test() { + $_SERVER["REQUEST_URI"] = url::file("var/test/var/uploads/.htaccess"); + $controller = new File_Proxy_Controller(); + try { + $controller->__call("", array()); + $this->assert_true(false); + } catch (Kohana_404_Exception $e) { + $this->assert_same(2, $e->test_fail_code); + } + } + + public function movie_thumbnails_are_jpgs_test() { + $movie = test::random_movie(); + $name = legal_file::change_extension($movie->name, "jpg"); + $_SERVER["REQUEST_URI"] = url::file("var/thumbs/{$movie->name}"); + $controller = new File_Proxy_Controller(); + $this->assert_same($movie->thumb_path(), $controller->__call("", array())); + } + + public function invalid_item_test() { + $photo = test::random_photo(); + $_SERVER["REQUEST_URI"] = url::file("var/albums/x_{$photo->name}"); + $controller = new File_Proxy_Controller(); + try { + $controller->__call("", array()); + $this->assert_true(false); + } catch (Kohana_404_Exception $e) { + $this->assert_same(3, $e->test_fail_code); + } + } + + public function need_view_full_permission_to_view_original_test() { + $album = test::random_album(); + $photo = test::random_photo($album); + $album = $album->reload(); // adding the photo changed the album in the db + $_SERVER["REQUEST_URI"] = url::file("var/albums/{$album->name}/{$photo->name}"); + $controller = new File_Proxy_Controller(); + + access::deny(identity::everybody(), "view_full", $album); + identity::set_active_user(identity::guest()); + + try { + $controller->__call("", array()); + $this->assert_true(false); + } catch (Kohana_404_Exception $e) { + $this->assert_same(5, $e->test_fail_code); + } + } + + public function cant_proxy_an_album_test() { + $album = test::random_album(); + $_SERVER["REQUEST_URI"] = url::file("var/albums/{$album->name}"); + $controller = new File_Proxy_Controller(); + + try { + $controller->__call("", array()); + $this->assert_true(false); + } catch (Kohana_404_Exception $e) { + $this->assert_same(6, $e->test_fail_code); + } + } + + public function missing_file_test() { + $photo = test::random_photo(); + $_SERVER["REQUEST_URI"] = url::file("var/albums/{$photo->name}"); + unlink($photo->file_path()); + $controller = new File_Proxy_Controller(); + + try { + $controller->__call("", array()); + $this->assert_true(false); + } catch (Kohana_404_Exception $e) { + $this->assert_same(7, $e->test_fail_code); + } + } +} \ No newline at end of file -- cgit v1.2.3 From d45a73777935c86fc5131955831833d7465b5e9d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 21 Jan 2013 01:22:01 -0500 Subject: Update copyright to 2013. Fixes #1953. --- application/Bootstrap.php | 2 +- application/config/config.php | 2 +- index.php | 2 +- installer/cli.php | 2 +- installer/index.php | 2 +- installer/installer.php | 2 +- installer/web.php | 2 +- modules/akismet/controllers/admin_akismet.php | 2 +- modules/akismet/helpers/akismet.php | 2 +- modules/akismet/helpers/akismet_event.php | 2 +- modules/akismet/helpers/akismet_installer.php | 2 +- modules/akismet/tests/Akismet_Helper_Test.php | 2 +- modules/comment/controllers/admin_comments.php | 2 +- modules/comment/controllers/admin_manage_comments.php | 2 +- modules/comment/controllers/comments.php | 2 +- modules/comment/helpers/comment.php | 2 +- modules/comment/helpers/comment_block.php | 2 +- modules/comment/helpers/comment_event.php | 2 +- modules/comment/helpers/comment_installer.php | 2 +- modules/comment/helpers/comment_rest.php | 2 +- modules/comment/helpers/comment_rss.php | 2 +- modules/comment/helpers/comment_theme.php | 2 +- modules/comment/helpers/comments_rest.php | 2 +- modules/comment/helpers/item_comments_rest.php | 2 +- modules/comment/models/comment.php | 2 +- modules/comment/tests/Comment_Event_Test.php | 2 +- modules/comment/tests/Comment_Helper_Test.php | 2 +- modules/comment/tests/Comment_Model_Test.php | 2 +- modules/digibug/config/digibug.php | 2 +- modules/digibug/controllers/admin_digibug.php | 2 +- modules/digibug/controllers/digibug.php | 2 +- modules/digibug/helpers/digibug_event.php | 2 +- modules/digibug/helpers/digibug_installer.php | 2 +- modules/digibug/helpers/digibug_theme.php | 2 +- modules/digibug/models/digibug_proxy.php | 2 +- modules/digibug/tests/Digibug_Controller_Test.php | 2 +- modules/exif/controllers/exif.php | 2 +- modules/exif/helpers/exif.php | 2 +- modules/exif/helpers/exif_event.php | 2 +- modules/exif/helpers/exif_installer.php | 2 +- modules/exif/helpers/exif_task.php | 2 +- modules/exif/helpers/exif_theme.php | 2 +- modules/exif/models/exif_key.php | 2 +- modules/exif/models/exif_record.php | 2 +- modules/exif/tests/Exif_Test.php | 2 +- modules/g2_import/controllers/admin_g2_import.php | 2 +- modules/g2_import/controllers/g2.php | 2 +- modules/g2_import/helpers/g2_import.php | 2 +- modules/g2_import/helpers/g2_import_event.php | 2 +- modules/g2_import/helpers/g2_import_installer.php | 2 +- modules/g2_import/helpers/g2_import_task.php | 2 +- modules/g2_import/libraries/G2_Import_Exception.php | 2 +- modules/g2_import/models/g2_map.php | 2 +- modules/gallery/config/cache.php | 2 +- modules/gallery/config/cookie.php | 2 +- modules/gallery/config/database.php | 2 +- modules/gallery/config/locale.php | 2 +- modules/gallery/config/log_file.php | 2 +- modules/gallery/config/routes.php | 2 +- modules/gallery/config/session.php | 2 +- modules/gallery/config/upload.php | 2 +- modules/gallery/config/user_agents.php | 2 +- modules/gallery/controllers/admin.php | 2 +- modules/gallery/controllers/admin_advanced_settings.php | 2 +- modules/gallery/controllers/admin_dashboard.php | 2 +- modules/gallery/controllers/admin_graphics.php | 2 +- modules/gallery/controllers/admin_languages.php | 2 +- modules/gallery/controllers/admin_maintenance.php | 2 +- modules/gallery/controllers/admin_modules.php | 2 +- modules/gallery/controllers/admin_sidebar.php | 2 +- modules/gallery/controllers/admin_theme_options.php | 2 +- modules/gallery/controllers/admin_themes.php | 2 +- modules/gallery/controllers/admin_upgrade_checker.php | 2 +- modules/gallery/controllers/albums.php | 2 +- modules/gallery/controllers/combined.php | 2 +- modules/gallery/controllers/file_proxy.php | 2 +- modules/gallery/controllers/items.php | 2 +- modules/gallery/controllers/l10n_client.php | 2 +- modules/gallery/controllers/login.php | 2 +- modules/gallery/controllers/logout.php | 2 +- modules/gallery/controllers/movies.php | 2 +- modules/gallery/controllers/packager.php | 2 +- modules/gallery/controllers/permissions.php | 2 +- modules/gallery/controllers/photos.php | 2 +- modules/gallery/controllers/quick.php | 2 +- modules/gallery/controllers/reauthenticate.php | 2 +- modules/gallery/controllers/upgrader.php | 2 +- modules/gallery/controllers/uploader.php | 2 +- modules/gallery/controllers/user_profile.php | 2 +- modules/gallery/controllers/welcome_message.php | 2 +- modules/gallery/helpers/MY_html.php | 2 +- modules/gallery/helpers/MY_num.php | 2 +- modules/gallery/helpers/MY_remote.php | 2 +- modules/gallery/helpers/MY_url.php | 2 +- modules/gallery/helpers/MY_valid.php | 2 +- modules/gallery/helpers/access.php | 2 +- modules/gallery/helpers/ajax.php | 2 +- modules/gallery/helpers/album.php | 2 +- modules/gallery/helpers/auth.php | 2 +- modules/gallery/helpers/batch.php | 2 +- modules/gallery/helpers/block_manager.php | 2 +- modules/gallery/helpers/data_rest.php | 2 +- modules/gallery/helpers/dir.php | 2 +- modules/gallery/helpers/encoding.php | 2 +- modules/gallery/helpers/gallery.php | 2 +- modules/gallery/helpers/gallery_block.php | 2 +- modules/gallery/helpers/gallery_error.php | 2 +- modules/gallery/helpers/gallery_event.php | 2 +- modules/gallery/helpers/gallery_graphics.php | 2 +- modules/gallery/helpers/gallery_installer.php | 2 +- modules/gallery/helpers/gallery_rss.php | 2 +- modules/gallery/helpers/gallery_task.php | 2 +- modules/gallery/helpers/gallery_theme.php | 2 +- modules/gallery/helpers/graphics.php | 2 +- modules/gallery/helpers/identity.php | 2 +- modules/gallery/helpers/item.php | 2 +- modules/gallery/helpers/item_rest.php | 2 +- modules/gallery/helpers/items_rest.php | 2 +- modules/gallery/helpers/json.php | 2 +- modules/gallery/helpers/l10n_client.php | 2 +- modules/gallery/helpers/l10n_scanner.php | 2 +- modules/gallery/helpers/legal_file.php | 2 +- modules/gallery/helpers/locales.php | 2 +- modules/gallery/helpers/log.php | 2 +- modules/gallery/helpers/message.php | 2 +- modules/gallery/helpers/model_cache.php | 2 +- modules/gallery/helpers/module.php | 2 +- modules/gallery/helpers/movie.php | 2 +- modules/gallery/helpers/photo.php | 2 +- modules/gallery/helpers/random.php | 2 +- modules/gallery/helpers/site_status.php | 2 +- modules/gallery/helpers/system.php | 2 +- modules/gallery/helpers/task.php | 2 +- modules/gallery/helpers/theme.php | 2 +- modules/gallery/helpers/tree_rest.php | 2 +- modules/gallery/helpers/upgrade_checker.php | 2 +- modules/gallery/helpers/user_profile.php | 2 +- modules/gallery/helpers/xml.php | 2 +- modules/gallery/hooks/init_gallery.php | 2 +- modules/gallery/libraries/Admin_View.php | 2 +- modules/gallery/libraries/Block.php | 2 +- modules/gallery/libraries/Breadcrumb.php | 2 +- modules/gallery/libraries/Form_Script.php | 2 +- modules/gallery/libraries/Form_Uploadify.php | 2 +- modules/gallery/libraries/Form_Uploadify_buttons.php | 2 +- modules/gallery/libraries/Gallery_I18n.php | 2 +- modules/gallery/libraries/Gallery_View.php | 2 +- modules/gallery/libraries/IdentityProvider.php | 2 +- modules/gallery/libraries/InPlaceEdit.php | 2 +- modules/gallery/libraries/MY_Database.php | 2 +- modules/gallery/libraries/MY_Forge.php | 2 +- modules/gallery/libraries/MY_Input.php | 2 +- modules/gallery/libraries/MY_Kohana_Exception.php | 2 +- modules/gallery/libraries/MY_ORM.php | 2 +- modules/gallery/libraries/MY_View.php | 2 +- modules/gallery/libraries/Menu.php | 2 +- modules/gallery/libraries/ORM_MPTT.php | 2 +- modules/gallery/libraries/SafeString.php | 2 +- modules/gallery/libraries/Sendmail.php | 2 +- modules/gallery/libraries/Task_Definition.php | 2 +- modules/gallery/libraries/Theme_View.php | 2 +- modules/gallery/libraries/drivers/Cache/Database.php | 2 +- modules/gallery/libraries/drivers/IdentityProvider.php | 2 +- modules/gallery/models/access_cache.php | 2 +- modules/gallery/models/access_intent.php | 2 +- modules/gallery/models/cache.php | 2 +- modules/gallery/models/failed_auth.php | 2 +- modules/gallery/models/graphics_rule.php | 2 +- modules/gallery/models/incoming_translation.php | 2 +- modules/gallery/models/item.php | 2 +- modules/gallery/models/log.php | 2 +- modules/gallery/models/message.php | 2 +- modules/gallery/models/module.php | 2 +- modules/gallery/models/outgoing_translation.php | 2 +- modules/gallery/models/permission.php | 2 +- modules/gallery/models/task.php | 2 +- modules/gallery/models/theme.php | 2 +- modules/gallery/models/var.php | 2 +- modules/gallery/tests/Access_Helper_Test.php | 2 +- modules/gallery/tests/Albums_Controller_Test.php | 2 +- modules/gallery/tests/Breadcrumb_Test.php | 2 +- modules/gallery/tests/Cache_Test.php | 2 +- modules/gallery/tests/Controller_Auth_Test.php | 2 +- modules/gallery/tests/Data_Rest_Helper_Test.php | 2 +- modules/gallery/tests/Database_Test.php | 2 +- modules/gallery/tests/Dir_Helper_Test.php | 2 +- modules/gallery/tests/DrawForm_Test.php | 2 +- modules/gallery/tests/File_Proxy_Controller_Test.php | 2 +- modules/gallery/tests/File_Structure_Test.php | 4 ++-- modules/gallery/tests/Gallery_Filters.php | 2 +- modules/gallery/tests/Gallery_I18n_Test.php | 2 +- modules/gallery/tests/Gallery_Installer_Test.php | 2 +- modules/gallery/tests/Html_Helper_Test.php | 2 +- modules/gallery/tests/Input_Library_Test.php | 2 +- modules/gallery/tests/Item_Helper_Test.php | 2 +- modules/gallery/tests/Item_Model_Test.php | 2 +- modules/gallery/tests/Item_Rest_Helper_Test.php | 2 +- modules/gallery/tests/Items_Rest_Helper_Test.php | 2 +- modules/gallery/tests/Kohana_Exception_Test.php | 2 +- modules/gallery/tests/Legal_File_Helper_Test.php | 2 +- modules/gallery/tests/Locales_Helper_Test.php | 2 +- modules/gallery/tests/Menu_Test.php | 2 +- modules/gallery/tests/Movie_Helper_Test.php | 2 +- modules/gallery/tests/Num_Helper_Test.php | 2 +- modules/gallery/tests/ORM_MPTT_Test.php | 2 +- modules/gallery/tests/Photos_Controller_Test.php | 2 +- modules/gallery/tests/SafeString_Test.php | 2 +- modules/gallery/tests/Sendmail_Test.php | 2 +- modules/gallery/tests/System_Helper_Test.php | 2 +- modules/gallery/tests/Url_Security_Test.php | 2 +- modules/gallery/tests/Valid_Test.php | 2 +- modules/gallery/tests/Var_Test.php | 2 +- modules/gallery/tests/Xss_Security_Test.php | 2 +- modules/gallery_unit_test/controllers/gallery_unit_test.php | 2 +- modules/gallery_unit_test/helpers/MY_request.php | 2 +- modules/gallery_unit_test/helpers/test.php | 2 +- modules/gallery_unit_test/libraries/Gallery_Unit_Test_Case.php | 2 +- modules/image_block/controllers/image_block.php | 2 +- modules/image_block/helpers/image_block_block.php | 2 +- modules/image_block/helpers/image_block_installer.php | 2 +- modules/info/helpers/info_block.php | 2 +- modules/info/helpers/info_installer.php | 2 +- modules/info/helpers/info_theme.php | 2 +- modules/kohana23_compat/config/pagination.php | 2 +- modules/kohana23_compat/libraries/MY_Database_Builder.php | 2 +- modules/kohana23_compat/libraries/Pagination.php | 2 +- modules/notification/controllers/notification.php | 2 +- modules/notification/helpers/notification.php | 2 +- modules/notification/helpers/notification_event.php | 2 +- modules/notification/helpers/notification_installer.php | 2 +- modules/notification/models/pending_notification.php | 2 +- modules/notification/models/subscription.php | 2 +- modules/organize/controllers/organize.php | 2 +- modules/organize/helpers/organize_event.php | 2 +- modules/organize/helpers/organize_installer.php | 2 +- modules/recaptcha/controllers/admin_recaptcha.php | 2 +- modules/recaptcha/helpers/recaptcha.php | 2 +- modules/recaptcha/helpers/recaptcha_event.php | 2 +- modules/recaptcha/helpers/recaptcha_installer.php | 2 +- modules/recaptcha/helpers/recaptcha_theme.php | 2 +- modules/recaptcha/libraries/Form_Recaptcha.php | 2 +- modules/rest/controllers/rest.php | 2 +- modules/rest/helpers/registry_rest.php | 2 +- modules/rest/helpers/rest.php | 2 +- modules/rest/helpers/rest_event.php | 2 +- modules/rest/helpers/rest_installer.php | 2 +- modules/rest/libraries/Rest_Exception.php | 2 +- modules/rest/models/user_access_key.php | 2 +- modules/rest/tests/Rest_Controller_Test.php | 2 +- modules/rss/controllers/rss.php | 2 +- modules/rss/helpers/rss.php | 2 +- modules/rss/helpers/rss_block.php | 2 +- modules/search/controllers/search.php | 2 +- modules/search/helpers/search.php | 2 +- modules/search/helpers/search_event.php | 2 +- modules/search/helpers/search_installer.php | 2 +- modules/search/helpers/search_task.php | 2 +- modules/search/helpers/search_theme.php | 2 +- modules/search/models/search_record.php | 2 +- modules/server_add/controllers/admin_server_add.php | 2 +- modules/server_add/controllers/server_add.php | 2 +- modules/server_add/helpers/server_add.php | 2 +- modules/server_add/helpers/server_add_event.php | 2 +- modules/server_add/helpers/server_add_installer.php | 2 +- modules/server_add/helpers/server_add_theme.php | 2 +- modules/server_add/models/server_add_entry.php | 2 +- modules/slideshow/helpers/slideshow_event.php | 2 +- modules/slideshow/helpers/slideshow_installer.php | 2 +- modules/slideshow/helpers/slideshow_theme.php | 2 +- modules/tag/controllers/admin_tags.php | 2 +- modules/tag/controllers/tag.php | 2 +- modules/tag/controllers/tag_name.php | 2 +- modules/tag/controllers/tags.php | 2 +- modules/tag/helpers/item_tags_rest.php | 2 +- modules/tag/helpers/tag.php | 2 +- modules/tag/helpers/tag_block.php | 2 +- modules/tag/helpers/tag_event.php | 2 +- modules/tag/helpers/tag_installer.php | 2 +- modules/tag/helpers/tag_item_rest.php | 2 +- modules/tag/helpers/tag_items_rest.php | 2 +- modules/tag/helpers/tag_rest.php | 2 +- modules/tag/helpers/tag_rss.php | 2 +- modules/tag/helpers/tag_task.php | 2 +- modules/tag/helpers/tag_theme.php | 2 +- modules/tag/helpers/tags_rest.php | 2 +- modules/tag/models/tag.php | 2 +- modules/tag/tests/Tag_Item_Rest_Helper_Test.php | 2 +- modules/tag/tests/Tag_Rest_Helper_Test.php | 2 +- modules/tag/tests/Tag_Test.php | 2 +- modules/tag/tests/Tags_Rest_Helper_Test.php | 2 +- modules/user/config/identity.php | 2 +- modules/user/controllers/admin_users.php | 2 +- modules/user/controllers/password.php | 2 +- modules/user/controllers/users.php | 2 +- modules/user/helpers/group.php | 2 +- modules/user/helpers/user.php | 2 +- modules/user/helpers/user_event.php | 2 +- modules/user/helpers/user_installer.php | 2 +- modules/user/helpers/user_theme.php | 2 +- modules/user/libraries/drivers/IdentityProvider/Gallery.php | 2 +- modules/user/models/group.php | 2 +- modules/user/models/user.php | 2 +- modules/user/tests/No_Direct_ORM_Access_Test.php | 2 +- modules/user/tests/User_Groups_Test.php | 2 +- modules/user/tests/User_Installer_Test.php | 2 +- modules/watermark/controllers/admin_watermarks.php | 2 +- modules/watermark/helpers/watermark.php | 2 +- modules/watermark/helpers/watermark_event.php | 2 +- modules/watermark/helpers/watermark_installer.php | 2 +- 309 files changed, 310 insertions(+), 310 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/application/Bootstrap.php b/application/Bootstrap.php index 183705d9..93353b47 100644 --- a/application/Bootstrap.php +++ b/application/Bootstrap.php @@ -1,7 +1,7 @@ Date: Mon, 21 Jan 2013 10:45:34 +0100 Subject: #1954 - Skip buffer calls for unit tests of file_proxy and data_rest. Moved the "if (TEST_MODE)" statement before the buffer calls in file_proxy and data_rest. This has no impact on normal use, but will make the unit tests more compatible with different server/PHP configurations. Note: We do not have to skip setting the headers, which means we can build unit tests around them if we wish. --- modules/gallery/controllers/file_proxy.php | 25 ++++++++++++------------- modules/gallery/helpers/data_rest.php | 11 ++++++----- 2 files changed, 18 insertions(+), 18 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php index f29cdf98..a9e98ce1 100644 --- a/modules/gallery/controllers/file_proxy.php +++ b/modules/gallery/controllers/file_proxy.php @@ -147,22 +147,21 @@ class File_Proxy_Controller extends Controller { header("Content-Type: $item->mime_type"); } - // Don't use Kohana::close_buffers(false) here because that only closes all the buffers - // that Kohana started. We want to close *all* buffers at this point because otherwise we're - // going to buffer up whatever file we're proxying (and it may be very large). This may - // affect embedding or systems with PHP's output_buffering enabled. - while (ob_get_level()) { - Kohana_Log::add("error","".print_r(ob_get_level(),1)); - if (!@ob_end_clean()) { - // ob_end_clean() can return false if the buffer can't be removed for some reason - // (zlib output compression buffers sometimes cause problems). - break; - } - } - if (TEST_MODE) { return $file; } else { + // Don't use Kohana::close_buffers(false) here because that only closes all the buffers + // that Kohana started. We want to close *all* buffers at this point because otherwise we're + // going to buffer up whatever file we're proxying (and it may be very large). This may + // affect embedding or systems with PHP's output_buffering enabled. + while (ob_get_level()) { + Kohana_Log::add("error","".print_r(ob_get_level(),1)); + if (!@ob_end_clean()) { + // ob_end_clean() can return false if the buffer can't be removed for some reason + // (zlib output compression buffers sometimes cause problems). + break; + } + } readfile($file); } } diff --git a/modules/gallery/helpers/data_rest.php b/modules/gallery/helpers/data_rest.php index 8a3a159d..dc213510 100644 --- a/modules/gallery/helpers/data_rest.php +++ b/modules/gallery/helpers/data_rest.php @@ -57,13 +57,14 @@ class data_rest_Core { } else { header("Content-Type: $item->mime_type"); } - Kohana::close_buffers(false); - if (isset($p->encoding) && $p->encoding == "base64") { - print base64_encode(file_get_contents($file)); + if (TEST_MODE) { + return $file; } else { - if (TEST_MODE) { - return $file; + Kohana::close_buffers(false); + + if (isset($p->encoding) && $p->encoding == "base64") { + print base64_encode(file_get_contents($file)); } else { readfile($file); } -- cgit v1.2.3 From 86a2759062ee3dbf93f4c17c7499e3f506035047 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 24 Jan 2013 18:14:14 -0500 Subject: If profiling is enabled, dump out profiling data instead of proxied images so that we can see how efficient our proxying is. Follow-on for #1959. --- modules/gallery/controllers/file_proxy.php | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php index a9e98ce1..7e5d0038 100644 --- a/modules/gallery/controllers/file_proxy.php +++ b/modules/gallery/controllers/file_proxy.php @@ -129,6 +129,13 @@ class File_Proxy_Controller extends Controller { throw $e; } + if (gallery::show_profiler()) { + Profiler::enable(); + $profiler = new Profiler(); + $profiler->render(); + exit; + } + header("Content-Length: " . filesize($file)); header("Pragma:"); -- cgit v1.2.3 From 5fca371a616dba16f955087c4477ee229ee222d0 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Mon, 28 Jan 2013 23:31:18 +0100 Subject: #1945 - Extend legal_file helper functions. - Added get_types_by_extension function, which is a merged version of get...types_by_extension functions (similar to get_extensions). - Added optional extension argument to get...extensions functions similar to get...types_by_extension functions. - Added unit tests. Now, every legal_file function has one. - Restructured helper file to include caches. - Added array_unique to get...types (derived from get...types_by_extension, which can be many-to-one). - Edited server_add, uploader, and item model to use new functionality. --- modules/gallery/controllers/uploader.php | 2 +- modules/gallery/helpers/legal_file.php | 146 +++++++++++++++++------ modules/gallery/models/item.php | 13 +- modules/gallery/tests/Legal_File_Helper_Test.php | 57 +++++++++ modules/server_add/controllers/server_add.php | 8 +- 5 files changed, 176 insertions(+), 50 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php index b2ec5b55..55c65c95 100644 --- a/modules/gallery/controllers/uploader.php +++ b/modules/gallery/controllers/uploader.php @@ -69,7 +69,7 @@ class Uploader_Controller extends Controller { $path_info = @pathinfo($temp_filename); if (array_key_exists("extension", $path_info) && - in_array(strtolower($path_info["extension"]), legal_file::get_movie_extensions())) { + legal_file::get_movie_extensions($path_info["extension"])) { $item->type = "movie"; $item->save(); log::success("content", t("Added a movie"), diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index 5768cf14..ab9047c8 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -18,6 +18,13 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class legal_file_Core { + private static $photo_types_by_extension; + private static $movie_types_by_extension; + private static $photo_extensions; + private static $movie_extensions; + private static $photo_types; + private static $movie_types; + /** * Create a default list of allowed photo MIME types paired with their extensions and then let * modules modify it. This is an ordered map, mapping extensions to their MIME types. @@ -26,21 +33,24 @@ class legal_file_Core { * @param string $extension (opt.) - return MIME of extension; if not given, return complete array */ static function get_photo_types_by_extension($extension=null) { - $types_by_extension_wrapper = new stdClass(); - $types_by_extension_wrapper->types_by_extension = array( - "jpg" => "image/jpeg", "jpeg" => "image/jpeg", "gif" => "image/gif", "png" => "image/png"); - module::event("photo_types_by_extension", $types_by_extension_wrapper); + if (empty(self::$photo_types_by_extension)) { + $types_by_extension_wrapper = new stdClass(); + $types_by_extension_wrapper->types_by_extension = array( + "jpg" => "image/jpeg", "jpeg" => "image/jpeg", "gif" => "image/gif", "png" => "image/png"); + module::event("photo_types_by_extension", $types_by_extension_wrapper); + self::$photo_types_by_extension = $types_by_extension_wrapper->types_by_extension; + } if ($extension) { // return matching MIME type $extension = strtolower($extension); - if (isset($types_by_extension_wrapper->types_by_extension[$extension])) { - return $types_by_extension_wrapper->types_by_extension[$extension]; + if (isset(self::$photo_types_by_extension[$extension])) { + return self::$photo_types_by_extension[$extension]; } else { return null; } } else { // return complete array - return $types_by_extension_wrapper->types_by_extension; + return self::$photo_types_by_extension; } } @@ -52,53 +62,111 @@ class legal_file_Core { * @param string $extension (opt.) - return MIME of extension; if not given, return complete array */ static function get_movie_types_by_extension($extension=null) { - $types_by_extension_wrapper = new stdClass(); - $types_by_extension_wrapper->types_by_extension = array( - "flv" => "video/x-flv", "mp4" => "video/mp4", "m4v" => "video/x-m4v"); - module::event("movie_types_by_extension", $types_by_extension_wrapper); + if (empty(self::$movie_types_by_extension)) { + $types_by_extension_wrapper = new stdClass(); + $types_by_extension_wrapper->types_by_extension = array( + "flv" => "video/x-flv", "mp4" => "video/mp4", "m4v" => "video/x-m4v"); + module::event("movie_types_by_extension", $types_by_extension_wrapper); + self::$movie_types_by_extension = $types_by_extension_wrapper->types_by_extension; + } + if ($extension) { + // return matching MIME type + $extension = strtolower($extension); + if (isset(self::$movie_types_by_extension[$extension])) { + return self::$movie_types_by_extension[$extension]; + } else { + return null; + } + } else { + // return complete array + return self::$movie_types_by_extension; + } + } + + /** + * Create a merged list of all allowed photo and movie MIME types paired with their extensions. + * + * @param string $extension (opt.) - return MIME of extension; if not given, return complete array + */ + static function get_types_by_extension($extension=null) { + $types_by_extension = legal_file::get_photo_types_by_extension(); + if (movie::find_ffmpeg()) { + $types_by_extension = array_merge($types_by_extension, + legal_file::get_movie_types_by_extension()); + } if ($extension) { // return matching MIME type $extension = strtolower($extension); - if (isset($types_by_extension_wrapper->types_by_extension[$extension])) { - return $types_by_extension_wrapper->types_by_extension[$extension]; + if (isset($types_by_extension[$extension])) { + return $types_by_extension[$extension]; } else { return null; } } else { // return complete array - return $types_by_extension_wrapper->types_by_extension; + return $types_by_extension; } } /** * Create a default list of allowed photo extensions and then let modules modify it. + * + * @param string $extension (opt.) - return true if allowed; if not given, return complete array */ - static function get_photo_extensions() { - $extensions_wrapper = new stdClass(); - $extensions_wrapper->extensions = array_keys(legal_file::get_photo_types_by_extension()); - module::event("legal_photo_extensions", $extensions_wrapper); - return $extensions_wrapper->extensions; + static function get_photo_extensions($extension=null) { + if (empty(self::$photo_extensions)) { + $extensions_wrapper = new stdClass(); + $extensions_wrapper->extensions = array_keys(legal_file::get_photo_types_by_extension()); + module::event("legal_photo_extensions", $extensions_wrapper); + self::$photo_extensions = $extensions_wrapper->extensions; + } + if ($extension) { + // return true if in array, false if not + return in_array(strtolower($extension), self::$photo_extensions); + } else { + // return complete array + return self::$photo_extensions; + } } /** * Create a default list of allowed movie extensions and then let modules modify it. + * + * @param string $extension (opt.) - return true if allowed; if not given, return complete array */ - static function get_movie_extensions() { - $extensions_wrapper = new stdClass(); - $extensions_wrapper->extensions = array_keys(legal_file::get_movie_types_by_extension()); - module::event("legal_movie_extensions", $extensions_wrapper); - return $extensions_wrapper->extensions; + static function get_movie_extensions($extension=null) { + if (empty(self::$movie_extensions)) { + $extensions_wrapper = new stdClass(); + $extensions_wrapper->extensions = array_keys(legal_file::get_movie_types_by_extension()); + module::event("legal_movie_extensions", $extensions_wrapper); + self::$movie_extensions = $extensions_wrapper->extensions; + } + if ($extension) { + // return true if in array, false if not + return in_array(strtolower($extension), self::$movie_extensions); + } else { + // return complete array + return self::$movie_extensions; + } } /** * Create a merged list of all allowed photo and movie extensions. + * + * @param string $extension (opt.) - return true if allowed; if not given, return complete array */ - static function get_extensions() { + static function get_extensions($extension=null) { $extensions = legal_file::get_photo_extensions(); if (movie::find_ffmpeg()) { $extensions = array_merge($extensions, legal_file::get_movie_extensions()); } - return $extensions; + if ($extension) { + // return true if in array, false if not + return in_array(strtolower($extension), $extensions); + } else { + // return complete array + return $extensions; + } } /** @@ -119,10 +187,14 @@ class legal_file_Core { * (e.g. flv maps to video/x-flv by default, but video/flv is still legal). */ static function get_photo_types() { - $types_wrapper = new stdClass(); - $types_wrapper->types = array_values(legal_file::get_photo_types_by_extension()); - module::event("legal_photo_types", $types_wrapper); - return $types_wrapper->types; + if (empty(self::$photo_types)) { + $types_wrapper = new stdClass(); + // Need array_unique since types_by_extension can be many-to-one (e.g. jpeg and jpg). + $types_wrapper->types = array_unique(array_values(legal_file::get_photo_types_by_extension())); + module::event("legal_photo_types", $types_wrapper); + self::$photo_types = $types_wrapper->types; + } + return self::$photo_types; } /** @@ -131,11 +203,15 @@ class legal_file_Core { * (e.g. flv maps to video/x-flv by default, but video/flv is still legal). */ static function get_movie_types() { - $types_wrapper = new stdClass(); - $types_wrapper->types = array_values(legal_file::get_movie_types_by_extension()); - $types_wrapper->types[] = "video/flv"; - module::event("legal_movie_types", $types_wrapper); - return $types_wrapper->types; + if (empty(self::$movie_types)) { + $types_wrapper = new stdClass(); + // Need array_unique since types_by_extension can be many-to-one (e.g. jpeg and jpg). + $types_wrapper->types = array_unique(array_values(legal_file::get_movie_types_by_extension())); + $types_wrapper->types[] = "video/flv"; + module::event("legal_movie_types", $types_wrapper); + self::$movie_types = $types_wrapper->types; + } + return self::$movie_types; } /** diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index e4b997aa..60318c26 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -879,16 +879,9 @@ class Item_Model_Core extends ORM_MPTT { return; } - if ($this->is_photo()) { - if (!in_array(strtolower($ext), legal_file::get_photo_extensions())) { - $v->add_error("name", "illegal_data_file_extension"); - } - } - - if ($this->is_movie()) { - if (!in_array(strtolower($ext), legal_file::get_movie_extensions())) { - $v->add_error("name", "illegal_data_file_extension"); - } + if ($this->is_photo() && !legal_file::get_photo_extensions($ext) || + $this->is_movie() && !legal_file::get_movie_extensions($ext)) { + $v->add_error("name", "illegal_data_file_extension"); } } diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php index 5db99935..84a29a52 100644 --- a/modules/gallery/tests/Legal_File_Helper_Test.php +++ b/modules/gallery/tests/Legal_File_Helper_Test.php @@ -40,6 +40,63 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case { $this->assert_equal(3, count(legal_file::get_movie_types_by_extension())); } + public function get_types_by_extension_test() { + $this->assert_equal("image/jpeg", legal_file::get_types_by_extension("jpg")); // photo + $this->assert_equal("video/x-flv", legal_file::get_types_by_extension("FLV")); // movie + $this->assert_equal(null, legal_file::get_types_by_extension("php")); // invalid + $this->assert_equal(null, legal_file::get_types_by_extension("php.flv")); // invalid w/ . + + // No extension returns full array + $this->assert_equal(7, count(legal_file::get_types_by_extension())); + } + + public function get_photo_extensions_test() { + $this->assert_equal(true, legal_file::get_photo_extensions("jpg")); // regular + $this->assert_equal(true, legal_file::get_photo_extensions("JPG")); // all caps + $this->assert_equal(true, legal_file::get_photo_extensions("Png")); // some caps + $this->assert_equal(false, legal_file::get_photo_extensions("php")); // invalid + $this->assert_equal(false, legal_file::get_photo_extensions("php.jpg")); // invalid w/ . + + // No extension returns full array + $this->assert_equal(4, count(legal_file::get_photo_extensions())); + } + + public function get_movie_extensions_test() { + $this->assert_equal(true, legal_file::get_movie_extensions("flv")); // regular + $this->assert_equal(true, legal_file::get_movie_extensions("FLV")); // all caps + $this->assert_equal(true, legal_file::get_movie_extensions("Mp4")); // some caps + $this->assert_equal(false, legal_file::get_movie_extensions("php")); // invalid + $this->assert_equal(false, legal_file::get_movie_extensions("php.jpg")); // invalid w/ . + + // No extension returns full array + $this->assert_equal(3, count(legal_file::get_movie_extensions())); + } + + public function get_extensions_test() { + $this->assert_equal(true, legal_file::get_extensions("jpg")); // photo + $this->assert_equal(true, legal_file::get_extensions("FLV")); // movie + $this->assert_equal(false, legal_file::get_extensions("php")); // invalid + $this->assert_equal(false, legal_file::get_extensions("php.jpg")); // invalid w/ . + + // No extension returns full array + $this->assert_equal(7, count(legal_file::get_extensions())); + } + + public function get_filters_test() { + // All 7 extensions both uppercase and lowercase + $this->assert_equal(14, count(legal_file::get_filters())); + } + + public function get_photo_types_test() { + // Note that this is one *less* than photo extensions since jpeg and jpg have the same mime. + $this->assert_equal(3, count(legal_file::get_photo_types())); + } + + public function get_movie_types_test() { + // Note that this is one *more* than movie extensions since video/flv is added. + $this->assert_equal(4, count(legal_file::get_movie_types())); + } + public function change_extension_test() { $this->assert_equal("foo.jpg", legal_file::change_extension("foo.png", "jpg")); } diff --git a/modules/server_add/controllers/server_add.php b/modules/server_add/controllers/server_add.php index df55d8db..f6e0a944 100644 --- a/modules/server_add/controllers/server_add.php +++ b/modules/server_add/controllers/server_add.php @@ -61,7 +61,7 @@ class Server_Add_Controller extends Admin_Controller { } if (!is_dir($file)) { $ext = strtolower(pathinfo($file, PATHINFO_EXTENSION)); - if (!in_array($ext, legal_file::get_extensions())) { + if (!legal_file::get_extensions($ext)) { continue; } } @@ -169,7 +169,7 @@ class Server_Add_Controller extends Admin_Controller { foreach ($child_paths as $child_path) { if (!is_dir($child_path)) { $ext = strtolower(pathinfo($child_path, PATHINFO_EXTENSION)); - if (!in_array($ext, legal_file::get_extensions()) || !filesize($child_path)) { + if (!legal_file::get_extensions($ext) || !filesize($child_path)) { // Not importable, skip it. continue; } @@ -255,7 +255,7 @@ class Server_Add_Controller extends Admin_Controller { } else { try { $extension = strtolower(pathinfo($name, PATHINFO_EXTENSION)); - if (in_array($extension, legal_file::get_photo_extensions())) { + if (legal_file::get_photo_extensions($extension)) { $photo = ORM::factory("item"); $photo->type = "photo"; $photo->parent_id = $parent->id; @@ -265,7 +265,7 @@ class Server_Add_Controller extends Admin_Controller { $photo->owner_id = $owner_id; $photo->save(); $entry->item_id = $photo->id; - } else if (in_array($extension, legal_file::get_movie_extensions())) { + } else if (legal_file::get_movie_extensions($extension)) { $movie = ORM::factory("item"); $movie->type = "movie"; $movie->parent_id = $parent->id; -- cgit v1.2.3 From 1e4d75c12072b49c3469f18af13bcf3439afc6b0 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Wed, 30 Jan 2013 12:10:18 -0500 Subject: Improve the display context API to return a "siblings_callback" field containing a callback that returns all the siblings. Fixes #1975. --- modules/gallery/controllers/albums.php | 6 ++++++ modules/search/controllers/search.php | 24 +++++++++++++----------- modules/tag/controllers/tag.php | 1 + 3 files changed, 20 insertions(+), 11 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index e14fe347..d545b415 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -93,10 +93,16 @@ class Albums_Controller extends Items_Controller { "previous_item" => $previous_item, "next_item" => $next_item, "sibling_count" => $item->parent()->viewable()->children_count($where), + "siblings_callback" => array("Albums_Controller::get_siblings", array($item)), "parents" => $item->parents()->as_array(), "breadcrumbs" => Breadcrumb::array_from_item_parents($item)); } + static function get_siblings($item) { + // @todo consider creating Item_Model::siblings() if we use this more broadly. + return $item->parent()->viewable()->children(); + } + public function create($parent_id) { access::verify_csrf(); $album = ORM::factory("item", $parent_id); diff --git a/modules/search/controllers/search.php b/modules/search/controllers/search.php index f7f1d981..673a281f 100644 --- a/modules/search/controllers/search.php +++ b/modules/search/controllers/search.php @@ -54,8 +54,6 @@ class Search_Controller extends Controller { list ($count, $result) = search::search_within_album($q_with_more_terms, $album, $page_size, $offset); - $title = t("Search: %q", array("q" => $q_with_more_terms)); - $max_pages = max(ceil($count / $page_size), 1); $template = new Theme_View("page.html", "collection", "search"); @@ -77,28 +75,27 @@ class Search_Controller extends Controller { print $template; - item::set_display_context_callback( - "Search_Controller::get_display_context", $album->id, $title, $q_with_more_terms, $q); + item::set_display_context_callback("Search_Controller::get_display_context", $album, $q); } - static function get_display_context($item, $album_id, $title, $query_terms, $q) { - $album = ORM::factory("item", $album_id); - $position = search::get_position_within_album($item, $query_terms, $album); + static function get_display_context($item, $album, $q) { + $q_with_more_terms = search::add_query_terms($q); + $position = search::get_position_within_album($item, $q_with_more_terms, $album); if ($position > 1) { list ($count, $result_data) = - search::search_within_album($query_terms, $album, 3, $position - 2); + search::search_within_album($q_with_more_terms, $album, 3, $position - 2); list ($previous_item, $ignore, $next_item) = $result_data; } else { $previous_item = null; list ($count, $result_data) = - search::search_within_album($query_terms, $album, 1, $position); + search::search_within_album($q_with_more_terms, $album, 1, $position); list ($next_item) = $result_data; } $search_url = url::abs_site("search" . "?q=" . urlencode($q) . - "&album=" . urlencode($album_id) . + "&album=" . urlencode($album->id) . "&show={$item->id}"); $root = item::root(); @@ -106,9 +103,14 @@ class Search_Controller extends Controller { "previous_item" => $previous_item, "next_item" => $next_item, "sibling_count" => $count, + "siblings_callback" => array("Search_Controller::get_siblings", array($q, $album)), "breadcrumbs" => array( - Breadcrumb::instance($root->title, "/", $root->id)->set_first(), + Breadcrumb::instance($root->title, $root->url())->set_first(), Breadcrumb::instance(t("Search: %q", array("q" => $q)), $search_url), Breadcrumb::instance($item->title, $item->url())->set_last())); } + + static function get_siblings($q, $album) { + return search::search_within_album(search::add_query_terms($q), $album, 1000, 1)[1]; + } } diff --git a/modules/tag/controllers/tag.php b/modules/tag/controllers/tag.php index 6199c49b..bada9bac 100644 --- a/modules/tag/controllers/tag.php +++ b/modules/tag/controllers/tag.php @@ -86,6 +86,7 @@ class Tag_Controller extends Controller { "previous_item" => $previous_item, "next_item" => $next_item, "sibling_count" => $tag->items_count($where), + "siblings_callback" => array(array($tag, "items"), array()), "breadcrumbs" => array( Breadcrumb::instance($root->title, $root->url())->set_first(), Breadcrumb::instance(t("Tag: %tag_name", array("tag_name" => $tag->name)), -- cgit v1.2.3 From ea54a88ec8d3c6e412f5efda58601006af1cf86c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Thu, 31 Jan 2013 16:29:09 -0500 Subject: Escape the host/username/password arguments to mysqldump. Fixes #1984. --- modules/gallery/controllers/packager.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'modules/gallery/controllers') diff --git a/modules/gallery/controllers/packager.php b/modules/gallery/controllers/packager.php index c48965b5..d7e3cf41 100644 --- a/modules/gallery/controllers/packager.php +++ b/modules/gallery/controllers/packager.php @@ -88,14 +88,17 @@ class Packager_Controller extends Controller { $dbconfig = Kohana::config('database.default'); $conn = $dbconfig["connection"]; - $pass = $conn["pass"] ? "-p{$conn['pass']}" : ""; $sql_file = DOCROOT . "installer/install.sql"; if (!is_writable($sql_file)) { print "$sql_file is not writeable"; return; } - $command = "mysqldump --compact --skip-extended-insert --add-drop-table -h{$conn['host']} " . - "-u{$conn['user']} $pass {$conn['database']} > $sql_file"; + $command = sprintf( + "mysqldump --compact --skip-extended-insert --add-drop-table %s %s %s %s > $sql_file", + escapeshellarg("-h{$conn['host']}"), + escapeshellarg("-u{$conn['user']}"), + $conn['pass'] ? escapeshellarg("-p{$conn['pass']}") : "", + escapeshellarg($conn['database'])); exec($command, $output, $status); if ($status) { print "
";
-- 
cgit v1.2.3


From 2dc97fab053996dcb5fe743329e1b786ca43712d Mon Sep 17 00:00:00 2001
From: shadlaws 
Date: Fri, 8 Feb 2013 09:03:57 +0100
Subject: #1993 - Remove "failed" query string from upgrader redirect if there
 are no errors. - Changed redirect if it finished without failures. - No
 change to Upgrader_Controller::index(), since its behavior with an empty vs.
 undefined failed query is identical.

---
 modules/gallery/controllers/upgrader.php | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/upgrader.php b/modules/gallery/controllers/upgrader.php
index e60385d9..d3c6e2ec 100644
--- a/modules/gallery/controllers/upgrader.php
+++ b/modules/gallery/controllers/upgrader.php
@@ -107,7 +107,11 @@ class Upgrader_Controller extends Controller {
         print "Upgrade complete\n";
       }
     } else {
-      url::redirect("upgrader?failed=" . join(",", $failed));
+      if ($failed) {
+        url::redirect("upgrader?failed=" . join(",", $failed));
+      } else {
+        url::redirect("upgrader");
+      }
     }
   }
 }
-- 
cgit v1.2.3


From 9dbe2e15ad9db3b2be53e46bc5f67d382fb4089d Mon Sep 17 00:00:00 2001
From: Bharat Mediratta 
Date: Sat, 9 Feb 2013 14:53:34 -0500
Subject: Extend siblings callbacks to take a $limit and an $offset for
 navigating large sibling sets.  Useful for the thumbnav module since we don't
 want to iterate a thousand siblings to find the one we care about.  Fixes
 #1999.

---
 modules/gallery/controllers/albums.php   | 4 ++--
 modules/gallery/libraries/Theme_View.php | 6 ++++--
 modules/search/controllers/search.php    | 4 ++--
 3 files changed, 8 insertions(+), 6 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php
index d545b415..0fb033a8 100644
--- a/modules/gallery/controllers/albums.php
+++ b/modules/gallery/controllers/albums.php
@@ -98,9 +98,9 @@ class Albums_Controller extends Items_Controller {
                  "breadcrumbs" => Breadcrumb::array_from_item_parents($item));
   }
 
-  static function get_siblings($item) {
+  static function get_siblings($item, $limit=null, $offset=null) {
     // @todo consider creating Item_Model::siblings() if we use this more broadly.
-    return $item->parent()->viewable()->children();
+    return $item->parent()->viewable()->children($limit, $offset);
   }
 
   public function create($parent_id) {
diff --git a/modules/gallery/libraries/Theme_View.php b/modules/gallery/libraries/Theme_View.php
index cf384109..986fc8a2 100644
--- a/modules/gallery/libraries/Theme_View.php
+++ b/modules/gallery/libraries/Theme_View.php
@@ -86,8 +86,10 @@ class Theme_View_Core extends Gallery_View {
     return $this->item;
   }
 
-  public function siblings() {
-    return call_user_func_array($this->siblings_callback[0], $this->siblings_callback[1]);
+  public function siblings($limit=null, $offset=null) {
+    return call_user_func_array(
+      $this->siblings_callback[0],
+      array_merge($this->siblings_callback[1], array($offset, $limit)));
   }
 
   public function tag() {
diff --git a/modules/search/controllers/search.php b/modules/search/controllers/search.php
index b54d7699..25ccd81c 100644
--- a/modules/search/controllers/search.php
+++ b/modules/search/controllers/search.php
@@ -110,8 +110,8 @@ class Search_Controller extends Controller {
                    Breadcrumb::instance($item->title, $item->url())->set_last()));
   }
 
-  static function get_siblings($q, $album) {
-    $result = search::search_within_album(search::add_query_terms($q), $album, 1000, 1);
+  static function get_siblings($q, $album, $limit=1000, $offset=1) {
+    $result = search::search_within_album(search::add_query_terms($q), $album, $limit, $offset);
     return $result[1];
   }
 }
-- 
cgit v1.2.3


From d04a6fc87d96b70ab0f70414f2ff40d1f1e7f482 Mon Sep 17 00:00:00 2001
From: shadlaws 
Date: Tue, 12 Feb 2013 00:37:33 +0100
Subject: #2001 - Make filename sanitizing more consistent. - legal_file -
 added sanitize_filname() to sanitize photo/movie filenames. -
 admin_watermarks - revised add() to use new function. - item model - added
 _process_data_file_info() to validate the data file, get its metadata, and
 sanitize the item name. - item model - revised save() for new items to use
 _process_data_file_info *before* the slug is checked. - item model - revised
 save() for updated items to use _process_data_file_info. - item model -
 revised save() for updated items to sanitize name if changed. - uploader -
 removed call to smash_extensions (item model does this when it calls
 sanitize_filename). - Legal_File_Helper_Test - added unit tests for
 sanitize_filename. - Item_Model_Test - revised existing unit tests based on
 changes. - Item_Model_Test - added new unit tests for names with legal but
 incorrect extensions. - Averted take over by HAL with fix #2001...

---
 modules/gallery/controllers/uploader.php           |   4 -
 modules/gallery/helpers/legal_file.php             |  57 ++++++++++++
 modules/gallery/models/item.php                    |  95 ++++++++++---------
 modules/gallery/tests/Item_Model_Test.php          | 101 ++++++++++-----------
 modules/gallery/tests/Legal_File_Helper_Test.php   |  44 +++++++++
 modules/watermark/controllers/admin_watermarks.php |   9 +-
 6 files changed, 202 insertions(+), 108 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php
index 55c65c95..78437071 100644
--- a/modules/gallery/controllers/uploader.php
+++ b/modules/gallery/controllers/uploader.php
@@ -63,10 +63,6 @@ class Uploader_Controller extends Controller {
         $item->parent_id = $album->id;
         $item->set_data_file($temp_filename);
 
-        // Remove double extensions from the filename - they'll be disallowed in the model but if
-        // we don't do it here then it'll result in a failed upload.
-        $item->name = legal_file::smash_extensions($item->name);
-
         $path_info = @pathinfo($temp_filename);
         if (array_key_exists("extension", $path_info) &&
             legal_file::get_movie_extensions($path_info["extension"])) {
diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index ef588ceb..5a852f2b 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -250,4 +250,61 @@ class legal_file_Core {
     $result .= isset($parts["extension"]) ? "{$parts['filename']}.{$parts['extension']}" : $parts["filename"];
     return $result;
   }
+
+  /**
+   * Sanitize a filename for a given type (given as "photo" or "movie") and a target file format
+   * (given as an extension).  This returns a completely legal and valid filename,
+   * or throws an exception if the type or extension given is invalid or illegal.  It tries to
+   * maintain the filename's original extension even if it's not identical to the given extension
+   * (e.g. don't change "JPG" or "jpeg" to "jpg").
+   *
+   * Note: it is not okay if the extension given is legal but does not match the type (e.g. if
+   * extension is "mp4" and type is "photo", it will throw an exception)
+   *
+   * @param  string $filename  (with no directory)
+   * @param  string $extension (can be uppercase or lowercase)
+   * @param  string $type      (as "photo" or "movie")
+   * @return string sanitized filename (or null if bad extension argument)
+   */
+  static function sanitize_filename($filename, $extension, $type) {
+    // Check if the type is valid - if so, get the mime types of the
+    // original and target extensions; if not, throw an exception.
+    $original_extension = pathinfo($filename, PATHINFO_EXTENSION);
+    switch ($type) {
+      case "photo":
+        $mime_type = legal_file::get_photo_types_by_extension($extension);
+        $original_mime_type = legal_file::get_photo_types_by_extension($original_extension);
+        break;
+      case "movie":
+        $mime_type = legal_file::get_movie_types_by_extension($extension);
+        $original_mime_type = legal_file::get_movie_types_by_extension($original_extension);
+        break;
+      default:
+        throw new Exception("@todo INVALID_TYPE");
+    }
+
+    // Check if the target extension is blank or invalid - if so, throw an exception.
+    if (!$extension || !$mime_type) {
+      throw new Exception("@todo ILLEGAL_EXTENSION");
+    }
+
+    // Check if the mime types of the original and target extensions match - if not, fix it.
+    if (!$original_extension || ($mime_type != $original_mime_type)) {
+      $filename = legal_file::change_extension($filename, $extension);
+    }
+
+    // It should be a filename without a directory - remove all slashes (and backslashes).
+    $filename = str_replace("/", "_", $filename);
+    $filename = str_replace("\\", "_", $filename);
+
+    // Remove extra dots from the filename.  This will also remove extraneous underscores.
+    $filename = legal_file::smash_extensions($filename);
+
+    // It's possible that the filename has no base (e.g. ".jpg") - if so, give it a generic one.
+    if (empty($filename) || (substr($filename, 0, 1) == ".")) {
+      $filename = $type . $filename;  // e.g. "photo.jpg" or "movie.mp4"
+    }
+
+    return $filename;
+  }
 }
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 33b8a89d..43b9a292 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -365,6 +365,14 @@ class Item_Model_Core extends ORM_MPTT {
           $this->weight = item::get_max_weight();
         }
 
+        // Process the data file info.
+        if (isset($this->data_file)) {
+          $this->_process_data_file_info();
+        } else if (!$this->is_album()) {
+          // Unless it's an album, new items must have a data file.
+          $this->data_file_error = true;
+        }
+
         // Make an url friendly slug from the name, if necessary
         if (empty($this->slug)) {
           $this->slug = item::convert_filename_to_slug(pathinfo($this->name, PATHINFO_FILENAME));
@@ -377,31 +385,6 @@ class Item_Model_Core extends ORM_MPTT {
           }
         }
 
-        // Get the width, height and mime type from our data file for photos and movies.
-        if ($this->is_photo() || $this->is_movie()) {
-          try {
-            if ($this->is_photo()) {
-              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);
-            }
-
-            // Force an extension onto the name if necessary
-            $pi = pathinfo($this->data_file);
-            if (empty($pi["extension"])) {
-              $this->name = "{$this->name}.$extension";
-            }
-
-            // Data file valid - make sure the flag is reset to false.
-            $this->data_file_error = false;
-          } catch (Exception $e) {
-            // Data file invalid - set the flag so it's reported during item validation.
-            $this->data_file_error = true;
-          }
-        }
-
         $this->_check_and_fix_conflicts();
 
         parent::save();
@@ -439,31 +422,19 @@ class Item_Model_Core extends ORM_MPTT {
         // keep it around.
         $original = ORM::factory("item", $this->id);
 
-        // Preserve the extension of the data file. Many helpers, (e.g. ImageMagick), assume
+        // If we have a new data file, process its info.  This will get its metadata and
+        // 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)) {
-          try {
-            $extension = pathinfo($this->data_file, PATHINFO_EXTENSION);
-            $new_name = pathinfo($this->name, PATHINFO_FILENAME) . ".$extension";
-            if (!empty($extension) && strcmp($this->name, $new_name)) {
-              $this->name = $new_name;
-            }
-            if ($this->is_photo()) {
-              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);
-            }
-            // Data file valid - make sure the flag is reset to false.
-            $this->data_file_error = false;
-          } catch (Exception $e) {
-            // Data file invalid - set the flag so it's reported during item validation.
-            $this->data_file_error = true;
-          }
+          $this->_process_data_file_info();
+        } else if (!$this->is_album() && array_key_exists("name", $this->changed)) {
+          // There's no new data file, but the name changed.  If it's a photo or movie,
+          // make sure the new name still agrees with the file type.
+          $this->name = legal_file::sanitize_filename($this->name,
+            pathinfo($original->name, PATHINFO_EXTENSION), $this->type);
         }
 
         // If an album's cover has changed (or been removed), delete any existing album cover,
@@ -624,6 +595,40 @@ class Item_Model_Core extends ORM_MPTT {
     }
   }
 
+  /**
+   * Process the data file info.  Get its metadata and extension.
+   * If valid, use it to sanitize the item name and update the
+   * width, height, and mime type.
+   */
+  private function _process_data_file_info() {
+    try {
+      if ($this->is_photo()) {
+        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);
+      } else {
+        // Albums don't have data files.
+        $this->data_file = null;
+        return;
+      }
+
+      // Sanitize the name based on the idenified extension, but only set $this->name if different
+      // to ensure it isn't unnecessarily marked as "changed"
+      $name = legal_file::sanitize_filename($this->name, $extension, $this->type);
+      if ($this->name != $name) {
+        $this->name = $name;
+      }
+
+      // Data file valid - make sure the flag is reset to false.
+      $this->data_file_error = false;
+    } catch (Exception $e) {
+      // Data file invalid - set the flag so it's reported during item validation.
+      $this->data_file_error = true;
+    }
+  }
+
   /**
    * Return the Item_Model representing the cover for this album.
    * @return Item_Model or null if there's no cover
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index a93498dd..fcb5c2ad 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -126,14 +126,9 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
 
   public function item_rename_wont_accept_slash_test() {
     $item = test::random_photo();
-    try {
-      $item->name = test::random_name() . "/";
-      $item->save();
-    } catch (ORM_Validation_Exception $e) {
-      $this->assert_equal(array("name" => "no_slashes"), $e->validation->errors());
-      return;
-    }
-    $this->assert_true(false, "Shouldn't get here");
+    $item->name = "/no_slashes/allowed/";
+    $item->save();
+    $this->assert_equal("no_slashes_allowed.jpg", $item->name);
   }
 
   public function move_album_test() {
@@ -328,30 +323,17 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
   }
 
   public function photo_files_must_have_an_extension_test() {
-    try {
-      $photo = test::random_photo_unsaved();
-      $photo->mime_type = "image/jpeg";
-      $photo->name = "no_extension";
-      $photo->save();
-    } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "illegal_data_file_extension"), $e->validation->errors());
-      return;  // pass
-    }
-    $this->assert_true(false, "Shouldn't get here");
+    $photo = test::random_photo_unsaved();
+    $photo->name = "no_extension_photo";
+    $photo->save();
+    $this->assert_equal("no_extension_photo.jpg", $photo->name);
   }
 
   public function movie_files_must_have_an_extension_test() {
-    try {
-      $movie = test::random_movie_unsaved();
-      $movie->type = "movie";
-      $movie->mime_type = "video/x-flv";
-      $movie->name = "no_extension";
-      $movie->save();
-    } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "illegal_data_file_extension"), $e->validation->errors());
-      return;  // pass
-    }
-    $this->assert_true(false, "Shouldn't get here");
+    $movie = test::random_movie_unsaved();
+    $movie->name = "no_extension_movie";
+    $movie->save();
+    $this->assert_equal("no_extension_movie.flv", $movie->name);
   }
 
   public function cant_delete_root_album_test() {
@@ -445,7 +427,7 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
       $photo->set_data_file(MODPATH . "gallery/tests/Item_Model_Test.php");
       $photo->save();
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "illegal_data_file_extension"), $e->validation->errors());
+      $this->assert_same(array("name" => "invalid_data_file"), $e->validation->errors());
       return;  // pass
     }
     $this->assert_true(false, "Shouldn't get here");
@@ -462,6 +444,7 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
       $this->assert_same(array("name" => "invalid_data_file"), $e->validation->errors());
       return;  // pass
     }
+    $this->assert_true(false, "Shouldn't get here");
   }
 
   public function urls_test() {
@@ -493,43 +476,55 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
       $album->thumb_url() . " is malformed");
   }
 
-  public function legal_extension_test() {
-    foreach (array("test.gif", "test.GIF", "test.Gif", "test.jpeg", "test.JPG") as $name) {
+  public function legal_extension_that_does_match_gets_used_test() {
+    foreach (array("jpg", "JPG", "Jpg", "jpeg") as $extension) {
       $photo = test::random_photo_unsaved(item::root());
-      $photo->name = $name;
+      $photo->name = test::random_name() . ".{$extension}";
       $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal($extension, pathinfo($photo->name, PATHINFO_EXTENSION));
     }
   }
 
   public function illegal_extension_test() {
     foreach (array("test.php", "test.PHP", "test.php5", "test.php4",
                    "test.pl", "test.php.png") as $name) {
-      try {
-        $photo = test::random_photo_unsaved(item::root());
-        $photo->name = $name;
-        $photo->save();
-      } catch (ORM_Validation_Exception $e) {
-        $this->assert_equal(array("name" => "illegal_data_file_extension"),
-                            $e->validation->errors());
-        continue;
-      }
-      $this->assert_true(false, "Shouldn't get here");
+      $photo = test::random_photo_unsaved(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
     }
   }
 
   public function cant_rename_to_illegal_extension_test() {
     foreach (array("test.php.test", "test.php", "test.PHP",
                    "test.php5", "test.php4", "test.pl") as $name) {
-      try {
-        $photo = test::random_photo(item::root());
-        $photo->name = $name;
-        $photo->save();
-      } catch (ORM_Validation_Exception $e) {
-        $this->assert_equal(array("name" => "illegal_data_file_extension"),
-                            $e->validation->errors());
-        continue;
-      }
-      $this->assert_true(false, "Shouldn't get here");
+      $photo = test::random_photo(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
+    }
+  }
+
+  public function legal_extension_that_doesnt_match_gets_fixed_test() {
+    foreach (array("test.png", "test.mp4", "test.GIF") as $name) {
+      $photo = test::random_photo_unsaved(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
+    }
+  }
+
+  public function rename_to_legal_extension_that_doesnt_match_gets_fixed_test() {
+    foreach (array("test.png", "test.mp4", "test.GIF") as $name) {
+      $photo = test::random_photo(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
     }
   }
 
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index 203d5616..7ed5214b 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -150,4 +150,48 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal("", legal_file::smash_extensions(""));
     $this->assert_equal(null, legal_file::smash_extensions(null));
   }
+
+  public function sanitize_filename_with_no_rename_test() {
+    $this->assert_equal("foo.jpeg", legal_file::sanitize_filename("foo.jpeg", "jpg", "photo"));
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo.jpg", "jpeg", "photo"));
+    $this->assert_equal("foo.MP4", legal_file::sanitize_filename("foo.MP4", "mp4", "movie"));
+    $this->assert_equal("foo.mp4", legal_file::sanitize_filename("foo.mp4", "MP4", "movie"));
+  }
+
+  public function sanitize_filename_with_corrected_extension_test() {
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo.png", "jpg", "photo"));
+    $this->assert_equal("foo.MP4", legal_file::sanitize_filename("foo.jpg", "MP4", "movie"));
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo.php", "jpg", "photo"));
+  }
+
+  public function sanitize_filename_with_non_standard_chars_and_dots_test() {
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo", "jpg", "photo"));
+    $this->assert_equal("foo.mp4", legal_file::sanitize_filename("foo.", "mp4", "movie"));
+    $this->assert_equal("foo.jpeg", legal_file::sanitize_filename(".foo.jpeg", "jpg", "photo"));
+    $this->assert_equal("foo_2013_02_10.jpeg",
+      legal_file::sanitize_filename("foo.2013/02/10.jpeg", "jpg", "photo"));
+    $this->assert_equal("foo_bar_baz.jpg",
+      legal_file::sanitize_filename("...foo...bar..baz...png", "jpg", "photo"));
+    $this->assert_equal("j'écris@un#nom_bizarre(mais quand_même_ça_passe.jpg",
+      legal_file::sanitize_filename("/j'écris@un#nom/bizarre(mais quand.même/ça_passe.\$ÇÀ@€#_", "jpg", "photo"));
+  }
+
+  public function sanitize_filename_with_no_base_name_test() {
+    $this->assert_equal("photo.jpg", legal_file::sanitize_filename(".png", "jpg", "photo"));
+    $this->assert_equal("movie.mp4", legal_file::sanitize_filename("__..__", "mp4", "movie"));
+    $this->assert_equal("photo.jpg", legal_file::sanitize_filename(".", "jpg", "photo"));
+    $this->assert_equal("movie.mp4", legal_file::sanitize_filename(null, "mp4", "movie"));
+  }
+
+  public function sanitize_filename_with_invalid_arguments_test() {
+    foreach (array("flv" => "photo", "jpg" => "movie", "php" => "photo",
+                   null => "movie", "jpg" => "album", "jpg" => null) as $extension => $type) {
+      try {
+        legal_file::sanitize_filename("foo.jpg", $extension, $type);
+        $this->assert_true(false, "Shouldn't get here");
+      } catch (Exception $e) {
+        // pass
+      }
+    }
+  }
 }
\ No newline at end of file
diff --git a/modules/watermark/controllers/admin_watermarks.php b/modules/watermark/controllers/admin_watermarks.php
index 59bb7fa9..b058d6a5 100644
--- a/modules/watermark/controllers/admin_watermarks.php
+++ b/modules/watermark/controllers/admin_watermarks.php
@@ -97,18 +97,15 @@ class Admin_Watermarks_Controller extends Admin_Controller {
     // validation logic will correctly reject it.  So, we skip validation when we're running tests.
     if (TEST_MODE || $form->validate()) {
       $file = $_POST["file"];
-      $pathinfo = pathinfo($file);
       // Forge prefixes files with "uploadfile-xxxxxxx" for uniqueness
-      $name = preg_replace("/uploadfile-[^-]+-(.*)/", '$1', $pathinfo["basename"]);
-      $name = legal_file::smash_extensions($name);
+      $name = preg_replace("/uploadfile-[^-]+-(.*)/", '$1', basename($file));
 
       try {
         list ($width, $height, $mime_type, $extension) = photo::get_file_metadata($file);
-        // Force correct, legal extension type on file, which will be of our canonical type
-        // (i.e. all lowercase, jpg instead of jpeg, etc.).  This renaming prevents the issues
+        // Sanitize filename, which ensures a valid extension.  This renaming prevents the issues
         // addressed in ticket #1855, where an image that looked valid (header said jpg) with a
         // php extension was previously accepted without changing its extension.
-        $name = legal_file::change_extension($name, $extension);
+        $name = legal_file::sanitize_filename($name, $extension, "photo");
       } catch (Exception $e) {
         message::error(t("Invalid or unidentifiable image file"));
         @unlink($file);
-- 
cgit v1.2.3