summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--.build_number2
-rw-r--r--modules/gallery/helpers/graphics.php10
-rw-r--r--modules/gallery/helpers/movie.php86
-rw-r--r--modules/gallery/helpers/photo.php63
-rw-r--r--modules/gallery/tests/Movie_Helper_Test.php32
-rw-r--r--modules/gallery/tests/Photo_Helper_Test.php56
-rw-r--r--modules/watermark/controllers/admin_watermarks.php32
7 files changed, 219 insertions, 62 deletions
diff --git a/.build_number b/.build_number
index 7cc8b76a..ba00687c 100644
--- a/.build_number
+++ b/.build_number
@@ -3,4 +3,4 @@
; process. You don't need to edit it. In fact..
;
; DO NOT EDIT THIS FILE BY HAND!
-build_number=296
+build_number=297
diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php
index 51437d4b..0c5f8366 100644
--- a/modules/gallery/helpers/graphics.php
+++ b/modules/gallery/helpers/graphics.php
@@ -195,9 +195,8 @@ class graphics_Core {
} else {
copy(MODPATH . "gallery/images/missing_photo.png", $item->thumb_path());
}
- $dims = getimagesize($item->thumb_path());
- $item->thumb_width = $dims[0];
- $item->thumb_height = $dims[1];
+ list ($item->thumb_width, $item->thumb_height) =
+ photo::get_file_metadata($item->thumb_path());
}
if (!empty($ops["resize"])) {
@@ -206,9 +205,8 @@ class graphics_Core {
} else {
copy(MODPATH . "gallery/images/missing_photo.png", $item->resize_path());
}
- $dims = getimagesize($item->resize_path());
- $item->resize_width = $dims[0];
- $item->resize_height = $dims[1];
+ list ($item->resize_width, $item->resize_height) =
+ photo::get_file_metadata($item->resize_path());
}
$item->save();
} catch (Exception $e) {
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index 7e6a2e55..6844771b 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -123,41 +123,79 @@ class movie_Core {
/**
* Return the width, height, mime_type, extension and duration of the given movie file.
+ * Metadata is first generated using ffmpeg (or set to defaults if it fails),
+ * then can be modified by other modules using movie_get_file_metadata events.
+ *
+ * This function and its use cases are symmetric to those of photo::get_file_metadata.
+ *
+ * @param string $file_path
+ * @return array array($width, $height, $mime_type, $extension, $duration)
+ *
+ * Use cases in detail:
+ * Input is standard movie type (flv/mp4/m4v)
+ * -> return metadata from ffmpeg
+ * Input is *not* standard movie type that is supported by ffmpeg (e.g. avi, mts...)
+ * -> return metadata from ffmpeg
+ * Input is *not* standard movie type that is *not* supported by ffmpeg but is legal
+ * -> return zero width, height, and duration; mime type and extension according to legal_file
+ * Input is *not* standard movie type that is *not* supported by ffmpeg and is *not* legal
+ * -> return zero width, height, and duration; null mime type and extension
+ * Input is not readable or does not exist
+ * -> throw exception
+ * Note: movie_get_file_metadata events can change any of the above cases (except the last one).
*/
static function get_file_metadata($file_path) {
- $ffmpeg = movie::find_ffmpeg();
- if (empty($ffmpeg)) {
- throw new Exception("@todo MISSING_FFMPEG");
+ if (!is_readable($file_path)) {
+ throw new Exception("@todo UNREADABLE_FILE");
}
- $cmd = escapeshellcmd($ffmpeg) . " -i " . escapeshellarg($file_path) . " 2>&1";
- $result = `$cmd`;
- if (preg_match("/Stream.*?Video:.*?, (\d+)x(\d+)/", $result, $matches_res)) {
- if (preg_match("/Stream.*?Video:.*? \[.*?DAR (\d+):(\d+).*?\]/", $result, $matches_dar) &&
- $matches_dar[1] >= 1 && $matches_dar[2] >= 1) {
- // DAR is defined - determine width based on height and DAR
- // (should always be int, but adding round to be sure)
- $matches_res[1] = round($matches_res[2] * $matches_dar[1] / $matches_dar[2]);
+ $metadata = new stdClass();
+ $ffmpeg = movie::find_ffmpeg();
+ if (!empty($ffmpeg)) {
+ // ffmpeg found - use it to get width, height, and duration.
+ $cmd = escapeshellcmd($ffmpeg) . " -i " . escapeshellarg($file_path) . " 2>&1";
+ $result = `$cmd`;
+ if (preg_match("/Stream.*?Video:.*?, (\d+)x(\d+)/", $result, $matches_res)) {
+ if (preg_match("/Stream.*?Video:.*? \[.*?DAR (\d+):(\d+).*?\]/", $result, $matches_dar) &&
+ $matches_dar[1] >= 1 && $matches_dar[2] >= 1) {
+ // DAR is defined - determine width based on height and DAR
+ // (should always be int, but adding round to be sure)
+ $matches_res[1] = round($matches_res[2] * $matches_dar[1] / $matches_dar[2]);
+ }
+ list ($metadata->width, $metadata->height) = array($matches_res[1], $matches_res[2]);
+ } else {
+ list ($metadata->width, $metadata->height) = array(0, 0);
+ }
+
+ if (preg_match("/Duration: (\d+:\d+:\d+\.\d+)/", $result, $matches)) {
+ $metadata->duration = movie::hhmmssdd_to_seconds($matches[1]);
+ } else if (preg_match("/duration.*?:.*?(\d+)/", $result, $matches)) {
+ $metadata->duration = $matches[1];
+ } else {
+ $metadata->duration = 0;
}
- list ($width, $height) = array($matches_res[1], $matches_res[2]);
} else {
- list ($width, $height) = array(0, 0);
+ // ffmpeg not found - set width, height, and duration to zero.
+ $metadata->width = 0;
+ $metadata->height = 0;
+ $metadata->duration = 0;
}
- $extension = strtolower(pathinfo($file_path, PATHINFO_EXTENSION));
- $extension = $extension ? $extension : "flv"; // No extension? Assume FLV.
- $mime_type = legal_file::get_movie_types_by_extension($extension);
- $mime_type = $mime_type ? $mime_type : "video/x-flv"; // No MIME found? Default to video/x-flv.
-
- if (preg_match("/Duration: (\d+:\d+:\d+\.\d+)/", $result, $matches)) {
- $duration = movie::hhmmssdd_to_seconds($matches[1]);
- } else if (preg_match("/duration.*?:.*?(\d+)/", $result, $matches)) {
- $duration = $matches[1];
+ $extension = pathinfo($file_path, PATHINFO_EXTENSION);
+ if (!$extension ||
+ (!$metadata->mime_type = legal_file::get_movie_types_by_extension($extension))) {
+ // Extension is empty or illegal.
+ $metadata->extension = null;
+ $metadata->mime_type = null;
} else {
- $duration = 0;
+ // Extension is legal (and mime is already set above).
+ $metadata->extension = strtolower($extension);
}
- return array($width, $height, $mime_type, $extension, $duration);
+ // Run movie_get_file_metadata events which can modify the class, then return results.
+ module::event("movie_get_file_metadata", $file_path, $metadata);
+ return array($metadata->width, $metadata->height, $metadata->mime_type,
+ $metadata->extension, $metadata->duration);
}
/**
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 855cd0ae..51e51507 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -80,20 +80,61 @@ class photo_Core {
/**
* Return the width, height, mime_type and extension of the given image file.
+ * Metadata is first generated using getimagesize (or the legal_file mapping if it fails),
+ * then can be modified by other modules using photo_get_file_metadata events.
+ *
+ * This function and its use cases are symmetric to those of photo::get_file_metadata.
+ *
+ * @param string $file_path
+ * @return array array($width, $height, $mime_type, $extension)
+ *
+ * Use cases in detail:
+ * Input is standard photo type (jpg/png/gif)
+ * -> return metadata from getimagesize()
+ * Input is *not* standard photo type that is supported by getimagesize (e.g. tif, bmp...)
+ * -> return metadata from getimagesize()
+ * Input is *not* standard photo type that is *not* supported by getimagesize but is legal
+ * -> return zero width and height, mime type and extension according to legal_file
+ * Input is *not* standard photo type that is *not* supported by getimagesize and is *not* legal
+ * -> return zero width and height, null mime type and extension
+ * Input is not readable or does not exist
+ * -> throw exception
+ * Note: photo_get_file_metadata events can change any of the above cases (except the last one).
*/
static function get_file_metadata($file_path) {
- $image_info = getimagesize($file_path);
- if ($image_info) {
- $width = $image_info[0];
- $height = $image_info[1];
- $mime_type = $image_info["mime"];
- $extension = image_type_to_extension($image_info[2], false);
- return array($width, $height, $mime_type, $extension);
+ if (!is_readable($file_path)) {
+ throw new Exception("@todo UNREADABLE_FILE");
+ }
+
+ $metadata = new stdClass();
+ if ($image_info = getimagesize($file_path)) {
+ // getimagesize worked - use its results.
+ $metadata->width = $image_info[0];
+ $metadata->height = $image_info[1];
+ $metadata->mime_type = $image_info["mime"];
+ $metadata->extension = image_type_to_extension($image_info[2], false);
+ // We prefer jpg instead of jpeg (which is returned by image_type_to_extension).
+ if ($metadata->extension == "jpeg") {
+ $metadata->extension = "jpg";
+ }
} else {
- // getimagesize failed - use legal_file mapping instead.
- $extension = strtolower(pathinfo($file_path, PATHINFO_EXTENSION));
- $mime_type = legal_file::get_photo_types_by_extension($extension);
- return array(0, 0, $mime_type, $extension);
+ // getimagesize failed - try to use legal_file mapping instead.
+ $extension = pathinfo($file_path, PATHINFO_EXTENSION);
+ if (!$extension ||
+ (!$metadata->mime_type = legal_file::get_photo_types_by_extension($extension))) {
+ // Extension is empty or illegal.
+ $metadata->extension = null;
+ $metadata->mime_type = null;
+ } else {
+ // Extension is legal (and mime is already set above).
+ $metadata->extension = strtolower($extension);
+ }
+ $metadata->width = 0;
+ $metadata->height = 0;
}
+
+ // Run photo_get_file_metadata events which can modify the class, then return results.
+ module::event("photo_get_file_metadata", $file_path, $metadata);
+ return array($metadata->width, $metadata->height, $metadata->mime_type, $metadata->extension);
}
}
diff --git a/modules/gallery/tests/Movie_Helper_Test.php b/modules/gallery/tests/Movie_Helper_Test.php
index ff7f798c..0c262620 100644
--- a/modules/gallery/tests/Movie_Helper_Test.php
+++ b/modules/gallery/tests/Movie_Helper_Test.php
@@ -46,4 +46,36 @@ class Movie_Helper_Test extends Gallery_Unit_Test_Case {
$this->assert_equal($seconds, movie::hhmmssdd_to_seconds($hhmmssdd));
}
}
+
+ public function get_file_metadata_test() {
+ $movie = test::random_movie();
+ $this->assert_equal(array(360, 288, "video/x-flv", "flv", 6.00),
+ movie::get_file_metadata($movie->file_path()));
+ }
+
+ public function get_file_metadata_with_non_existent_file_test() {
+ try {
+ $metadata = movie::get_file_metadata(MODPATH . "gallery/tests/this_does_not_exist");
+ $this->assert_true(false, "Shouldn't get here");
+ } catch (Exception $e) {
+ // pass
+ }
+ }
+
+ public function get_file_metadata_with_no_extension_test() {
+ copy(MODPATH . "gallery/tests/test.flv", TMPPATH . "test_flv_with_no_extension");
+ $this->assert_equal(array(360, 288, null, null, 6.00),
+ movie::get_file_metadata(TMPPATH . "test_flv_with_no_extension"));
+ }
+
+ public function get_file_metadata_with_illegal_extension_test() {
+ $this->assert_equal(array(0, 0, null, null, 0),
+ movie::get_file_metadata(MODPATH . "gallery/tests/Movie_Helper_Test.php"));
+ }
+
+ public function get_file_metadata_with_illegal_extension_but_valid_file_contents_test() {
+ copy(MODPATH . "gallery/tests/test.flv", TMPPATH . "test_flv_with_php_extension.php");
+ $this->assert_equal(array(360, 288, null, null, 6.00),
+ movie::get_file_metadata(TMPPATH . "test_flv_with_php_extension.php"));
+ }
}
diff --git a/modules/gallery/tests/Photo_Helper_Test.php b/modules/gallery/tests/Photo_Helper_Test.php
new file mode 100644
index 00000000..5207a6db
--- /dev/null
+++ b/modules/gallery/tests/Photo_Helper_Test.php
@@ -0,0 +1,56 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2013 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+class Photo_Helper_Test extends Gallery_Unit_Test_Case {
+ public function get_file_metadata_test() {
+ $photo = test::random_photo();
+ $this->assert_equal(array(1024, 768, "image/jpeg", "jpg"),
+ photo::get_file_metadata($photo->file_path()));
+ }
+
+ public function get_file_metadata_with_non_existent_file_test() {
+ try {
+ $metadata = photo::get_file_metadata(MODPATH . "gallery/tests/this_does_not_exist");
+ $this->assert_true(false, "Shouldn't get here");
+ } catch (Exception $e) {
+ // pass
+ }
+ }
+
+ public function get_file_metadata_with_no_extension_test() {
+ copy(MODPATH . "gallery/tests/test.jpg", TMPPATH . "test_jpg_with_no_extension");
+ $this->assert_equal(array(1024, 768, "image/jpeg", "jpg"),
+ photo::get_file_metadata(TMPPATH . "test_jpg_with_no_extension"));
+ }
+
+ public function get_file_metadata_with_illegal_extension_test() {
+ $this->assert_equal(array(0, 0, null, null),
+ photo::get_file_metadata(MODPATH . "gallery/tests/Photo_Helper_Test.php"));
+ }
+
+ public function get_file_metadata_with_illegal_extension_but_valid_file_contents_test() {
+ // This ensures that we correctly "re-type" files with invalid extensions if the contents
+ // themselves are valid. This is needed to ensure that issues similar to those corrected by
+ // ticket #1855, where an image that looked valid (header said jpg) with a php extension was
+ // previously accepted without changing its extension, do not arise and cause security issues.
+ copy(MODPATH . "gallery/tests/test.jpg", TMPPATH . "test_jpg_with_php_extension.php");
+ $this->assert_equal(array(1024, 768, "image/jpeg", "jpg"),
+ photo::get_file_metadata(TMPPATH . "test_jpg_with_php_extension.php"));
+ }
+}
diff --git a/modules/watermark/controllers/admin_watermarks.php b/modules/watermark/controllers/admin_watermarks.php
index 0e6e214b..14c2b394 100644
--- a/modules/watermark/controllers/admin_watermarks.php
+++ b/modules/watermark/controllers/admin_watermarks.php
@@ -100,32 +100,24 @@ class Admin_Watermarks_Controller extends Admin_Controller {
$name = preg_replace("/uploadfile-[^-]+-(.*)/", '$1', $pathinfo["basename"]);
$name = legal_file::smash_extensions($name);
- if (!($image_info = getimagesize($file)) ||
- !in_array($image_info[2], array(IMAGETYPE_GIF, IMAGETYPE_JPEG, IMAGETYPE_PNG))) {
- message::error(t("Unable to identify this image file"));
+ list ($width, $height, $mime_type, $extension) = photo::get_file_metadata($file);
+ if (!legal_file::get_photo_extensions($extension)) {
+ message::error(t("Invalid or unidentifiable image file"));
@unlink($file);
return;
- }
-
- if (!in_array($pathinfo["extension"], legal_file::get_photo_extensions())) {
- switch ($image_info[2]) {
- case IMAGETYPE_GIF:
- $name = legal_file::change_extension($name, "gif");
- break;
- case IMAGETYPE_JPEG:
- $name = legal_file::change_extension($name, "jpg");
- break;
- case IMAGETYPE_PNG:
- $name = legal_file::change_extension($name, "png");
- break;
- }
+ } else {
+ // 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
+ // 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);
}
rename($file, VARPATH . "modules/watermark/$name");
module::set_var("watermark", "name", $name);
- module::set_var("watermark", "width", $image_info[0]);
- module::set_var("watermark", "height", $image_info[1]);
- module::set_var("watermark", "mime_type", $image_info["mime"]);
+ module::set_var("watermark", "width", $width);
+ module::set_var("watermark", "height", $height);
+ module::set_var("watermark", "mime_type", $mime_type);
module::set_var("watermark", "position", $form->add_watermark->position->value);
module::set_var("watermark", "transparency", $form->add_watermark->transparency->value);
$this->_update_graphics_rules();