From 3a9009492e6d05fe657e3aacf34cf40ca3495db5 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Fri, 18 Jan 2013 20:06:05 +0100 Subject: #1943 - Make legal_file::change_extension more robust. Previously would fail with dots in the directory but no extension. Added unit tests to verify the new change works. --- modules/gallery/tests/Legal_File_Helper_Test.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'modules/gallery/tests/Legal_File_Helper_Test.php') diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php index d80bcafe..d2813633 100644 --- a/modules/gallery/tests/Legal_File_Helper_Test.php +++ b/modules/gallery/tests/Legal_File_Helper_Test.php @@ -36,6 +36,24 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case { legal_file::change_extension("/website/foo.com/VID_20120513_105421.mp4", "jpg")); } + public function change_extension_path_containing_dots_and_no_extension_test() { + $this->assert_equal( + "/website/foo.com/VID_20120513_105421.jpg", + legal_file::change_extension("/website/foo.com/VID_20120513_105421", "jpg")); + } + + public function change_extension_path_containing_dots_and_dot_extension_test() { + $this->assert_equal( + "/website/foo.com/VID_20120513_105421.jpg", + legal_file::change_extension("/website/foo.com/VID_20120513_105421.", "jpg")); + } + + public function change_extension_path_containing_dots_and_non_standard_chars_test() { + $this->assert_equal( + "/j'écris@un#nom/bizarre(mais quand.même/ça_passe.jpg", + legal_file::change_extension("/j'écris@un#nom/bizarre(mais quand.même/ça_passe.\$ÇÀ@€#_", "jpg")); + } + public function smash_extensions_test() { $this->assert_equal("foo_bar.jpg", legal_file::smash_extensions("foo.bar.jpg")); $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("foo.bar.baz.jpg")); -- cgit v1.2.3 From e2a2a5ce812c249b46d56ed2f738162e3f4cd09b Mon Sep 17 00:00:00 2001 From: shadlaws Date: Sat, 19 Jan 2013 00:52:13 +0100 Subject: #1944 - Fix possible warnings in legal_file::get_photo_types_by_extension and legal_file::get_movie_types_by_extension. Added unit tests for these two functions, too. --- modules/gallery/helpers/legal_file.php | 6 ++-- modules/gallery/tests/Legal_File_Helper_Test.php | 37 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) (limited to 'modules/gallery/tests/Legal_File_Helper_Test.php') diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index e24aca36..70c07065 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -25,13 +25,14 @@ 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) { + 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 ($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]; } else { @@ -50,13 +51,14 @@ 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) { + 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 ($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]; } else { diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php index d2813633..66a5cc68 100644 --- a/modules/gallery/tests/Legal_File_Helper_Test.php +++ b/modules/gallery/tests/Legal_File_Helper_Test.php @@ -18,6 +18,43 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case { + public function get_photo_types_by_extension_test() { + // Valid extensions return their corresponding mimes, invalid extensions return null + $tests = array("jpg" => "image/jpeg", + "JPG" => "image/jpeg", + "jpeg" => "image/jpeg", + "png" => "image/png", + "Png" => "image/png", + "gif" => "image/gif", + "tif" => null, + "mp4" => null, + "php" => null, + "php.jpg" => null); + foreach ($tests as $extension => $mime_type) { + $this->assert_equal($mime_type, legal_file::get_photo_types_by_extension($extension)); + } + // No extension returns full array + $this->assert_equal(4, count(legal_file::get_photo_types_by_extension())); + } + + public function get_movie_types_by_extension_test() { + // Valid extensions return their corresponding mimes, invalid extensions return null + $tests = array("flv" => "video/x-flv", + "FLV" => "video/x-flv", + "mp4" => "video/mp4", + "Mp4" => "video/mp4", + "m4v" => "video/x-m4v", + "avi" => null, + "jpg" => null, + "php" => null, + "php.flv" => null); + foreach ($tests as $extension => $mime_type) { + $this->assert_equal($mime_type, legal_file::get_movie_types_by_extension($extension)); + } + // No extension returns full array + $this->assert_equal(3, count(legal_file::get_movie_types_by_extension())); + } + public function change_extension_test() { $this->assert_equal("foo.jpg", legal_file::change_extension("foo.png", "jpg")); } -- cgit v1.2.3 From a06abe32b0d12b83a5b1b1d5bd10b112a1e50465 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sun, 20 Jan 2013 00:39:58 -0500 Subject: Simplify get_photo_types_by_extension_test and get_movie_types_by_extension_test. --- modules/gallery/tests/Legal_File_Helper_Test.php | 39 ++++++++---------------- 1 file changed, 12 insertions(+), 27 deletions(-) (limited to 'modules/gallery/tests/Legal_File_Helper_Test.php') diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php index 66a5cc68..041c5d1f 100644 --- a/modules/gallery/tests/Legal_File_Helper_Test.php +++ b/modules/gallery/tests/Legal_File_Helper_Test.php @@ -19,38 +19,23 @@ */ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case { public function get_photo_types_by_extension_test() { - // Valid extensions return their corresponding mimes, invalid extensions return null - $tests = array("jpg" => "image/jpeg", - "JPG" => "image/jpeg", - "jpeg" => "image/jpeg", - "png" => "image/png", - "Png" => "image/png", - "gif" => "image/gif", - "tif" => null, - "mp4" => null, - "php" => null, - "php.jpg" => null); - foreach ($tests as $extension => $mime_type) { - $this->assert_equal($mime_type, legal_file::get_photo_types_by_extension($extension)); - } + $this->assert_equal("image/jpeg", legal_file::get_photo_types_by_extension("jpg")); // regular + $this->assert_equal("image/jpeg", legal_file::get_photo_types_by_extension("JPG")); // all caps + $this->assert_equal("image/png", legal_file::get_photo_types_by_extension("Png")); // some caps + $this->assert_equal(null, legal_file::get_photo_types_by_extension("php")); // invalid + $this->assert_equal(null, legal_file::get_photo_types_by_extension("php.jpg")); // invalid w/ . + // No extension returns full array $this->assert_equal(4, count(legal_file::get_photo_types_by_extension())); } public function get_movie_types_by_extension_test() { - // Valid extensions return their corresponding mimes, invalid extensions return null - $tests = array("flv" => "video/x-flv", - "FLV" => "video/x-flv", - "mp4" => "video/mp4", - "Mp4" => "video/mp4", - "m4v" => "video/x-m4v", - "avi" => null, - "jpg" => null, - "php" => null, - "php.flv" => null); - foreach ($tests as $extension => $mime_type) { - $this->assert_equal($mime_type, legal_file::get_movie_types_by_extension($extension)); - } + $this->assert_equal("video/x-flv", legal_file::get_movie_types_by_extension("flv")); // regular + $this->assert_equal("video/x-flv", legal_file::get_movie_types_by_extension("FLV")); // all caps + $this->assert_equal("video/mp4", legal_file::get_movie_types_by_extension("Mp4")); // some caps + $this->assert_equal(null, legal_file::get_movie_types_by_extension("php")); // invalid + $this->assert_equal(null, legal_file::get_movie_types_by_extension("php.flv")); // invalid w/ . + // No extension returns full array $this->assert_equal(3, count(legal_file::get_movie_types_by_extension())); } -- 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/tests/Legal_File_Helper_Test.php') 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, 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/tests/Legal_File_Helper_Test.php') 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 bfdf5a00fd8a256c4b564887f4eb649cf9d34774 Mon Sep 17 00:00:00 2001 From: shadlaws Date: Sun, 10 Feb 2013 02:28:47 +0100 Subject: #2000 - Make legal_file::smash_extensions more robust. - Fixed legal_file::smash_extensions. - Added new unit test. - Removed duplicated condition in existing unit test. --- modules/gallery/helpers/legal_file.php | 6 +++++- modules/gallery/tests/Legal_File_Helper_Test.php | 10 +++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'modules/gallery/tests/Legal_File_Helper_Test.php') diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php index 9ed564a1..ef588ceb 100644 --- a/modules/gallery/helpers/legal_file.php +++ b/modules/gallery/helpers/legal_file.php @@ -235,6 +235,10 @@ class legal_file_Core { * Reduce the given file to having a single extension. */ static function smash_extensions($filename) { + if (!$filename) { + // It's harmless, so return it before it causes issues with pathinfo. + return $filename; + } $parts = pathinfo($filename); $result = ""; if ($parts["dirname"] != ".") { @@ -243,7 +247,7 @@ class legal_file_Core { $parts["filename"] = str_replace(".", "_", $parts["filename"]); $parts["filename"] = preg_replace("/[_]+/", "_", $parts["filename"]); $parts["filename"] = trim($parts["filename"], "_"); - $result .= "{$parts['filename']}.{$parts['extension']}"; + $result .= isset($parts["extension"]) ? "{$parts['filename']}.{$parts['extension']}" : $parts["filename"]; return $result; } } diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php index 84a29a52..203d5616 100644 --- a/modules/gallery/tests/Legal_File_Helper_Test.php +++ b/modules/gallery/tests/Legal_File_Helper_Test.php @@ -136,10 +136,18 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case { public function smash_extensions_test() { $this->assert_equal("foo_bar.jpg", legal_file::smash_extensions("foo.bar.jpg")); $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("foo.bar.baz.jpg")); - $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("foo.bar.baz.jpg")); $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("...foo...bar..baz...jpg")); $this->assert_equal("/path/to/foo_bar.jpg", legal_file::smash_extensions("/path/to/foo.bar.jpg")); $this->assert_equal("/path/to.to/foo_bar.jpg", legal_file::smash_extensions("/path/to.to/foo.bar.jpg")); $this->assert_equal("foo_bar-12345678.jpg", legal_file::smash_extensions("foo.bar-12345678.jpg")); } + + public function smash_extensions_pass_thru_names_without_extensions_test() { + $this->assert_equal("foo", legal_file::smash_extensions("foo")); + $this->assert_equal("foo.", legal_file::smash_extensions("foo.")); + $this->assert_equal(".foo", legal_file::smash_extensions(".foo")); + $this->assert_equal(".", legal_file::smash_extensions(".")); + $this->assert_equal("", legal_file::smash_extensions("")); + $this->assert_equal(null, legal_file::smash_extensions(null)); + } } \ No newline at end of file -- 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/tests/Legal_File_Helper_Test.php') 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