From 23e7a3cf4074114db6f13fb8fe37134ecf7d689d Mon Sep 17 00:00:00 2001
From: Nathaniel <catch@35733.no-reply.drupal.org>
Date: Thu, 1 Dec 2011 16:59:16 +0900
Subject: [PATCH] Issue #1338428 by jbrown, BTMash, Tor Arne Thune: Fixed
image_dimensions_scale() and image_scale_effect() are ungrokable and buggy.
---
core/includes/image.inc | 42 ++++----
core/modules/image/image.effects.inc | 6 +-
core/modules/image/image.test | 141 +++++++++++++++++++++++++++
3 files changed, 161 insertions(+), 28 deletions(-)
diff --git a/core/includes/image.inc b/core/includes/image.inc
index 8dc36b995bc0..f6ae7f19b4c4 100644
--- a/core/includes/image.inc
+++ b/core/includes/image.inc
@@ -186,14 +186,14 @@ function image_scale_and_crop(stdClass $image, $width, $height) {
* Dimensions to be modified - an array with components width and height, in
* pixels.
* @param $width
- * The target width, in pixels. This value is omitted then the scaling will
+ * The target width, in pixels. If this value is NULL then the scaling will be
* based only on the height value.
* @param $height
- * The target height, in pixels. This value is omitted then the scaling will
- * based only on the width value.
+ * The target height, in pixels. If this value is NULL then the scaling will
+ * be based only on the width value.
* @param $upscale
- * Boolean indicating that files smaller than the dimensions will be scaled
- * up. This generally results in a low quality image.
+ * Boolean indicating that images smaller than the target dimensions will be
+ * scaled up. This generally results in a low quality image.
*
* @return
* TRUE if $dimensions was modified, FALSE otherwise.
@@ -203,31 +203,25 @@ function image_scale_and_crop(stdClass $image, $width, $height) {
function image_dimensions_scale(array &$dimensions, $width = NULL, $height = NULL, $upscale = FALSE) {
$aspect = $dimensions['height'] / $dimensions['width'];
- if ($upscale) {
- // Set width/height according to aspect ratio if either is empty.
- $width = !empty($width) ? $width : $height / $aspect;
- $height = !empty($height) ? $height : $width / $aspect;
+ // Calculate one of the dimensions from the other target dimension,
+ // ensuring the same aspect ratio as the source dimensions. If one of the
+ // target dimensions is missing, that is the one that is calculated. If both
+ // are specified then the dimension calculated is the one that would not be
+ // calculated to be bigger than its target.
+ if (($width && !$height) || ($width && $height && $aspect < $height / $width)) {
+ $height = (int) round($width * $aspect);
}
else {
- // Set impossibly large values if the width and height aren't set.
- $width = !empty($width) ? $width : 9999999;
- $height = !empty($height) ? $height : 9999999;
-
- // Don't scale up.
- if (round($width) >= $dimensions['width'] && round($height) >= $dimensions['height']) {
- return FALSE;
- }
+ $width = (int) round($height / $aspect);
}
- if ($aspect < $height / $width) {
- $dimensions['width'] = $width;
- $dimensions['height'] = (int) round($width * $aspect);
- }
- else {
- $dimensions['width'] = (int) round($height / $aspect);
- $dimensions['height'] = $height;
+ // Don't upscale if the option isn't enabled.
+ if (!$upscale && ($width >= $dimensions['width'] || $height >= $dimensions['height'])) {
+ return FALSE;
}
+ $dimensions['width'] = $width;
+ $dimensions['height'] = $height;
return TRUE;
}
diff --git a/core/modules/image/image.effects.inc b/core/modules/image/image.effects.inc
index ea898f91f0fc..35a6a74c7ac9 100644
--- a/core/modules/image/image.effects.inc
+++ b/core/modules/image/image.effects.inc
@@ -124,13 +124,11 @@ function image_resize_dimensions(array &$dimensions, array $data) {
function image_scale_effect(&$image, $data) {
// Set sane default values.
$data += array(
+ 'width' => NULL,
+ 'height' => NULL,
'upscale' => FALSE,
);
- // Set impossibly large values if the width and height aren't set.
- $data['width'] = empty($data['width']) ? PHP_INT_MAX : $data['width'];
- $data['height'] = empty($data['height']) ? PHP_INT_MAX : $data['height'];
-
if (!image_scale($image, $data['width'], $data['height'], $data['upscale'])) {
watchdog('image', 'Image scale failed using the %toolkit toolkit on %path (%mimetype, %dimensions)', array('%toolkit' => $image->toolkit, '%path' => $image->source, '%mimetype' => $image->info['mime_type'], '%dimensions' => $image->info['width'] . 'x' . $image->info['height']), WATCHDOG_ERROR);
return FALSE;
diff --git a/core/modules/image/image.test b/core/modules/image/image.test
index 70f69e727190..444c2ed2e10d 100644
--- a/core/modules/image/image.test
+++ b/core/modules/image/image.test
@@ -1129,3 +1129,144 @@ class ImageDimensionsUnitTest extends DrupalWebTestCase {
$this->assertEqual($img_tag, '<img typeof="foaf:Image" src="' . $url . '" alt="" />', t('Expected img tag was found.'));
}
}
+
+/**
+ * Tests image_dimensions_scale().
+ */
+class ImageDimensionsScaleTestCase extends DrupalUnitTestCase {
+ public static function getInfo() {
+ return array(
+ 'name' => 'image_dimensions_scale()',
+ 'description' => 'Tests all control flow branches in image_dimensions_scale().',
+ 'group' => 'Image',
+ );
+ }
+
+ /**
+ * Tests all control flow branches in image_dimensions_scale().
+ */
+ function testImageDimensionsScale() {
+ // Define input / output datasets to test different branch conditions.
+ $test = array();
+
+ // Test branch conditions:
+ // - No height.
+ // - Upscale, don't need to upscale.
+ $tests[] = array(
+ 'input' => array(
+ 'dimensions' => array(
+ 'width' => 1000,
+ 'height' => 2000,
+ ),
+ 'width' => 200,
+ 'height' => NULL,
+ 'upscale' => TRUE,
+ ),
+ 'output' => array(
+ 'dimensions' => array(
+ 'width' => 200,
+ 'height' => 400,
+ ),
+ 'return_value' => TRUE,
+ ),
+ );
+
+ // Test branch conditions:
+ // - No width.
+ // - Don't upscale, don't need to upscale.
+ $tests[] = array(
+ 'input' => array(
+ 'dimensions' => array(
+ 'width' => 1000,
+ 'height' => 800,
+ ),
+ 'width' => NULL,
+ 'height' => 140,
+ 'upscale' => FALSE,
+ ),
+ 'output' => array(
+ 'dimensions' => array(
+ 'width' => 175,
+ 'height' => 140,
+ ),
+ 'return_value' => TRUE,
+ ),
+ );
+
+ // Test branch conditions:
+ // - Source aspect ratio greater than target.
+ // - Upscale, need to upscale.
+ $tests[] = array(
+ 'input' => array(
+ 'dimensions' => array(
+ 'width' => 8,
+ 'height' => 20,
+ ),
+ 'width' => 200,
+ 'height' => 140,
+ 'upscale' => TRUE,
+ ),
+ 'output' => array(
+ 'dimensions' => array(
+ 'width' => 56,
+ 'height' => 140,
+ ),
+ 'return_value' => TRUE,
+ ),
+ );
+
+ // Test branch condition: target aspect ratio greater than source.
+ $tests[] = array(
+ 'input' => array(
+ 'dimensions' => array(
+ 'width' => 2000,
+ 'height' => 800,
+ ),
+ 'width' => 200,
+ 'height' => 140,
+ 'upscale' => FALSE,
+ ),
+ 'output' => array(
+ 'dimensions' => array(
+ 'width' => 200,
+ 'height' => 80,
+ ),
+ 'return_value' => TRUE,
+ ),
+ );
+
+ // Test branch condition: don't upscale, need to upscale.
+ $tests[] = array(
+ 'input' => array(
+ 'dimensions' => array(
+ 'width' => 100,
+ 'height' => 50,
+ ),
+ 'width' => 200,
+ 'height' => 140,
+ 'upscale' => FALSE,
+ ),
+ 'output' => array(
+ 'dimensions' => array(
+ 'width' => 100,
+ 'height' => 50,
+ ),
+ 'return_value' => FALSE,
+ ),
+ );
+
+ foreach ($tests as $test) {
+ // Process the test dataset.
+ $return_value = image_dimensions_scale($test['input']['dimensions'], $test['input']['width'], $test['input']['height'], $test['input']['upscale']);
+
+ // Check the width.
+ $this->assertEqual($test['output']['dimensions']['width'], $test['input']['dimensions']['width'], t('Computed width (@computed_width) equals expected width (@expected_width)', array('@computed_width' => $test['output']['dimensions']['width'], '@expected_width' => $test['input']['dimensions']['width'])));
+
+ // Check the height.
+ $this->assertEqual($test['output']['dimensions']['height'], $test['input']['dimensions']['height'], t('Computed height (@computed_height) equals expected height (@expected_height)', array('@computed_height' => $test['output']['dimensions']['height'], '@expected_height' => $test['input']['dimensions']['height'])));
+
+ // Check the return value.
+ $this->assertEqual($test['output']['return_value'], $return_value, t('Correct return value.'));
+ }
+ }
+}
--
GitLab