summaryrefslogtreecommitdiff
path: root/modules
diff options
context:
space:
mode:
Diffstat (limited to 'modules')
-rw-r--r--modules/g2_import/controllers/g2.php2
-rw-r--r--modules/gallery/controllers/albums.php4
-rw-r--r--modules/gallery/controllers/uploader.php4
-rw-r--r--modules/gallery/helpers/gallery_installer.php8
-rw-r--r--modules/gallery/helpers/legal_file.php67
-rw-r--r--modules/gallery/helpers/movie.php25
-rw-r--r--modules/gallery/helpers/system.php36
-rw-r--r--modules/gallery/libraries/Form_Uploadify.php2
-rw-r--r--modules/gallery/libraries/Theme_View.php6
-rw-r--r--modules/gallery/models/item.php95
-rw-r--r--modules/gallery/module.info2
-rw-r--r--modules/gallery/tests/Item_Model_Test.php101
-rw-r--r--modules/gallery/tests/Legal_File_Helper_Test.php54
-rw-r--r--modules/search/controllers/search.php4
-rw-r--r--modules/watermark/controllers/admin_watermarks.php9
15 files changed, 290 insertions, 129 deletions
diff --git a/modules/g2_import/controllers/g2.php b/modules/g2_import/controllers/g2.php
index c9b1d182..c24d52e9 100644
--- a/modules/g2_import/controllers/g2.php
+++ b/modules/g2_import/controllers/g2.php
@@ -37,7 +37,7 @@ class G2_Controller extends Controller {
// Tags did not have mappings created, so we need to catch them first. However, if a g2_itemId was
// passed, we'll want to show lookup the mapping anyway
- if (($path && 0 === strpos($path, "tag/")) || $view = "tags.VirtualAlbum") {
+ if (($path && 0 === strpos($path, "tag/")) || $view == "tags.VirtualAlbum") {
if (0 === strpos($path, "tag/")) {
$tag_name = substr($path, 4);
}
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/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/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index 7f10cdee..051a66cf 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -315,6 +315,7 @@ class gallery_installer {
module::set_var("gallery", "timezone", null);
module::set_var("gallery", "lock_timeout", 1);
module::set_var("gallery", "movie_extract_frame_time", 3);
+ module::set_var("gallery", "movie_allow_uploads", "autodetect");
}
static function upgrade($version) {
@@ -789,6 +790,13 @@ class gallery_installer {
module::set_version("gallery", $version = 55);
}
+ if ($version == 55) {
+ // In v56, we added the ability to change the default behavior regarding movie uploads. It
+ // can be set to "always", "never", or "autodetect" to match the previous behavior where they
+ // are allowed only if FFmpeg is found.
+ module::set_var("gallery", "movie_allow_uploads", "autodetect");
+ module::set_version("gallery", $version = 56);
+ }
}
static function uninstall() {
diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index 9ed564a1..eb9c25de 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -98,7 +98,7 @@ class legal_file_Core {
*/
static function get_types_by_extension($extension=null) {
$types_by_extension = legal_file::get_photo_types_by_extension();
- if (movie::find_ffmpeg()) {
+ if (movie::allow_uploads()) {
$types_by_extension = array_merge($types_by_extension,
legal_file::get_movie_types_by_extension());
}
@@ -165,7 +165,7 @@ class legal_file_Core {
*/
static function get_extensions($extension=null) {
$extensions = legal_file::get_photo_extensions();
- if (movie::find_ffmpeg()) {
+ if (movie::allow_uploads()) {
$extensions = array_merge($extensions, legal_file::get_movie_extensions());
}
if ($extension) {
@@ -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,64 @@ 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;
}
+
+ /**
+ * 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/helpers/movie.php b/modules/gallery/helpers/movie.php
index 637d8ac0..863e1cf9 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -24,6 +24,8 @@
* Note: by design, this class does not do any permission checking.
*/
class movie_Core {
+ private static $allow_uploads;
+
static function get_edit_form($movie) {
$form = new Forge("movies/update/$movie->id", "", "post", array("id" => "g-edit-movie-form"));
$form->hidden("from_id")->value($movie->id);
@@ -110,6 +112,29 @@ class movie_Core {
}
/**
+ * Return true if movie uploads are allowed, false if not. This is based on the
+ * "movie_allow_uploads" Gallery variable as well as whether or not ffmpeg is found.
+ */
+ static function allow_uploads() {
+ if (empty(self::$allow_uploads)) {
+ // Refresh ffmpeg settings
+ $ffmpeg = movie::find_ffmpeg();
+ switch (module::get_var("gallery", "movie_allow_uploads", "autodetect")) {
+ case "always":
+ self::$allow_uploads = true;
+ break;
+ case "never":
+ self::$allow_uploads = false;
+ break;
+ default:
+ self::$allow_uploads = !empty($ffmpeg);
+ break;
+ }
+ }
+ return self::$allow_uploads;
+ }
+
+ /**
* Return the path to the ffmpeg binary if one exists and is executable, or null.
*/
static function find_ffmpeg() {
diff --git a/modules/gallery/helpers/system.php b/modules/gallery/helpers/system.php
index fa834824..e1398103 100644
--- a/modules/gallery/helpers/system.php
+++ b/modules/gallery/helpers/system.php
@@ -20,22 +20,42 @@
class system_Core {
/**
* Return the path to an executable version of the named binary, or null.
- * Traverse the PATH environment variable looking for the given file. If
- * the $priority_path variable is set, check that path first.
+ * The paths are traversed in the following order:
+ * 1. $priority_path (if specified)
+ * 2. Gallery's own bin directory (DOCROOT . "bin")
+ * 3. PATH environment variable
+ * 4. extra_binary_paths Gallery variable (if specified)
+ * In addition, if the file is found inside Gallery's bin directory but
+ * it's not executable, we try to change its permissions to 0755.
+ *
+ * @param string $binary
+ * @param string $priority_path (optional)
+ * @return string path to binary if found; null if not found
*/
static function find_binary($binary, $priority_path=null) {
- $paths = array_merge(
- explode(":", getenv("PATH")),
- explode(":", module::get_var("gallery", "extra_binary_paths")));
+ $bin_path = DOCROOT . "bin";
+
if ($priority_path) {
- array_unshift($paths, $priority_path);
+ $paths = array($priority_path, $bin_path);
+ } else {
+ $paths = array($bin_path);
}
+ $paths = array_merge($paths,
+ explode(":", getenv("PATH")),
+ explode(":", module::get_var("gallery", "extra_binary_paths")));
foreach ($paths as $path) {
$candidate = "$path/$binary";
// @suppress errors below to avoid open_basedir issues
- if (@file_exists($candidate) && @is_executable($candidate)) {
- return $candidate;
+ if (@file_exists($candidate)) {
+ if (!@is_executable($candidate) &&
+ (substr_compare($bin_path, $candidate, 0, strlen($bin_path)) == 0)) {
+ // Binary isn't executable but is in Gallery's bin directory - try fixing permissions.
+ @chmod($candidate, 0755);
+ }
+ if (@is_executable($candidate)) {
+ return $candidate;
+ }
}
}
return null;
diff --git a/modules/gallery/libraries/Form_Uploadify.php b/modules/gallery/libraries/Form_Uploadify.php
index 56793c69..1e58018d 100644
--- a/modules/gallery/libraries/Form_Uploadify.php
+++ b/modules/gallery/libraries/Form_Uploadify.php
@@ -46,7 +46,7 @@ class Form_Uploadify_Core extends Form_Input {
$v->album = $this->data["album"];
$v->script_data = $this->data["script_data"];
$v->simultaneous_upload_limit = module::get_var("gallery", "simultaneous_upload_limit");
- $v->movies_allowed = (bool) movie::find_ffmpeg();
+ $v->movies_allowed = movie::allow_uploads();
$v->extensions = legal_file::get_filters();
$v->suhosin_session_encrypt = (bool) ini_get("suhosin.session.encrypt");
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/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/module.info b/modules/gallery/module.info
index d79a5077..2383ec3c 100644
--- a/modules/gallery/module.info
+++ b/modules/gallery/module.info
@@ -1,6 +1,6 @@
name = "Gallery 3"
description = "Gallery core application"
-version = 55
+version = 56
author_name = "Gallery Team"
author_url = "http://codex.galleryproject.org/Gallery:Team"
info_url = "http://codex.galleryproject.org/Gallery3:Modules:gallery"
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 84a29a52..7ed5214b 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -136,10 +136,62 @@ 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));
+ }
+
+ 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/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];
}
}
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);