diff options
Diffstat (limited to 'modules/gallery')
-rw-r--r-- | modules/gallery/controllers/uploader.php | 4 | ||||
-rw-r--r-- | modules/gallery/helpers/legal_file.php | 57 | ||||
-rw-r--r-- | modules/gallery/models/item.php | 95 | ||||
-rw-r--r-- | modules/gallery/tests/Item_Model_Test.php | 101 | ||||
-rw-r--r-- | modules/gallery/tests/Legal_File_Helper_Test.php | 44 |
5 files changed, 199 insertions, 102 deletions
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 debd1e6d..eb9c25de 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, @@ -625,6 +596,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 |