From 6f96ab3e9093d03088ebd6bde399edf2d1ba2fac Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 14 Mar 2014 11:13:45 +0100 Subject: [PATCH 1/7] make it possible to influence output type of \OC_Image --- lib/private/image.php | 47 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/private/image.php b/lib/private/image.php index a4a23f0f097..df49bf9848d 100644 --- a/lib/private/image.php +++ b/lib/private/image.php @@ -150,9 +150,12 @@ class OC_Image { * @brief Outputs the image. * @returns bool */ - public function show() { - header('Content-Type: '.$this->mimeType()); - return $this->_output(); + public function show($mimeType=null) { + if($mimeType === null) { + $mimeType = $this->mimeType(); + } + header('Content-Type: '.$mimeType); + return $this->_output(null, $mimeType); } /** @@ -161,20 +164,23 @@ class OC_Image { * @param string $filePath */ - public function save($filePath=null) { + public function save($filePath=null, $mimeType=null) { + if($mimeType === null) { + $mimeType = $this->mimeType(); + } if($filePath === null && $this->filePath === null) { OC_Log::write('core', __METHOD__.'(): called with no path.', OC_Log::ERROR); return false; } elseif($filePath === null && $this->filePath !== null) { $filePath = $this->filePath; } - return $this->_output($filePath); + return $this->_output($filePath, $mimeType); } /** * @brief Outputs/saves the image. */ - private function _output($filePath=null) { + private function _output($filePath=null, $mimeType=null) { if($filePath) { if (!file_exists(dirname($filePath))) mkdir(dirname($filePath), 0777, true); @@ -192,7 +198,34 @@ class OC_Image { return false; } - switch($this->imageType) { + $imageType = null; + if($mimeType !== null) { + switch($mimeType) { + case 'image/gif': + $this->imageType = IMAGETYPE_GIF; + break; + case 'image/jpeg': + case 'image/pjpeg': + $this->imageType = IMAGETYPE_JPEG; + break; + case 'image/png': + $this->imageType = IMAGETYPE_PNG; + break; + case 'image/x-xbitmap': + $this->imageType = IMAGETYPE_XBM; + break; + case 'image/bmp': + $this->imageType = IMAGETYPE_BMP; + break; + default: + $this->imageType = IMAGETYPE_PNG; + break; + } + } else { + $imageType = $this->imageType; + } + + switch($imageType) { case IMAGETYPE_GIF: $retVal = imagegif($this->resource, $filePath); break; From 0c3c75efd5fd2e68d67ac45a63e6226d1b8e822b Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 14 Mar 2014 11:14:09 +0100 Subject: [PATCH 2/7] always output a png --- lib/private/preview.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/preview.php b/lib/private/preview.php index 74051fbc2a3..70bfdb6e70a 100755 --- a/lib/private/preview.php +++ b/lib/private/preview.php @@ -463,7 +463,7 @@ class Preview { if (is_null($this->preview)) { $this->getPreview(); } - $this->preview->show(); + $this->preview->show('image/png'); return; } From c140010e813480c5d67ec7b7751b0b36a8a3af65 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 14 Mar 2014 11:17:20 +0100 Subject: [PATCH 3/7] don't change mimetype of whole image object --- lib/private/image.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/private/image.php b/lib/private/image.php index df49bf9848d..d108360b2a8 100644 --- a/lib/private/image.php +++ b/lib/private/image.php @@ -202,23 +202,23 @@ class OC_Image { if($mimeType !== null) { switch($mimeType) { case 'image/gif': - $this->imageType = IMAGETYPE_GIF; + $imageType = IMAGETYPE_GIF; break; case 'image/jpeg': case 'image/pjpeg': - $this->imageType = IMAGETYPE_JPEG; + $imageType = IMAGETYPE_JPEG; break; case 'image/png': - $this->imageType = IMAGETYPE_PNG; + $imageType = IMAGETYPE_PNG; break; case 'image/x-xbitmap': - $this->imageType = IMAGETYPE_XBM; + $imageType = IMAGETYPE_XBM; break; case 'image/bmp': - $this->imageType = IMAGETYPE_BMP; + $imageType = IMAGETYPE_BMP; break; default: - $this->imageType = IMAGETYPE_PNG; + $imageType = IMAGETYPE_PNG; break; } } else { From 9dee3772eb3c46a7a23efc83d77da6c5a3aba030 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 14 Mar 2014 18:12:33 +0100 Subject: [PATCH 4/7] remove image/pjpeg from OC_Image::_output --- lib/private/image.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/image.php b/lib/private/image.php index d108360b2a8..4aa14be109c 100644 --- a/lib/private/image.php +++ b/lib/private/image.php @@ -205,7 +205,6 @@ class OC_Image { $imageType = IMAGETYPE_GIF; break; case 'image/jpeg': - case 'image/pjpeg': $imageType = IMAGETYPE_JPEG; break; case 'image/png': From 3527adbbe1ab947ca6e6edbba889539eea444915 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Fri, 14 Mar 2014 18:19:16 +0100 Subject: [PATCH 5/7] OC_Image::_output() - throw exception instead of falling back to png --- lib/private/image.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/image.php b/lib/private/image.php index 4aa14be109c..1cd7399cce1 100644 --- a/lib/private/image.php +++ b/lib/private/image.php @@ -217,7 +217,7 @@ class OC_Image { $imageType = IMAGETYPE_BMP; break; default: - $imageType = IMAGETYPE_PNG; + throw new Exception('\OC_Image::_output(): "' . $mimeType . '" is not supported when forcing a specific output format'); break; } } else { From 79ba930ef90d3e54c02f62c43f61c190e11fb4ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 17 Mar 2014 08:17:56 +0100 Subject: [PATCH 6/7] fixing PHPDoc --- lib/private/image.php | 107 +++++++++++++++++++++++------------------- 1 file changed, 58 insertions(+), 49 deletions(-) diff --git a/lib/private/image.php b/lib/private/image.php index 1cd7399cce1..dd041442ed6 100644 --- a/lib/private/image.php +++ b/lib/private/image.php @@ -35,7 +35,7 @@ class OC_Image { /** * @brief Get mime type for an image file. * @param string|null $filepath The path to a local image file. - * @returns string The mime type if the it could be determined, otherwise an empty string. + * @return string The mime type if the it could be determined, otherwise an empty string. */ static public function getMimeTypeForFile($filePath) { // exif_imagetype throws "read error!" if file is less than 12 byte @@ -48,10 +48,11 @@ class OC_Image { } /** - * @brief Constructor. - * @param string|resource $imageref The path to a local file, a base64 encoded string or a resource created by an imagecreate* function. - * @returns bool False on error - */ + * @brief Constructor. + * @param resource|string $imageref The path to a local file, a base64 encoded string or a resource created by + * an imagecreate* function. + * @return \OC_Image False on error + */ public function __construct($imageRef = null) { //OC_Log::write('core',__METHOD__.'(): start', OC_Log::DEBUG); if(!extension_loaded('gd') || !function_exists('gd_info')) { @@ -70,7 +71,7 @@ class OC_Image { /** * @brief Determine whether the object contains an image resource. - * @returns bool + * @return bool */ public function valid() { // apparently you can't name a method 'empty'... return is_resource($this->resource); @@ -78,7 +79,7 @@ class OC_Image { /** * @brief Returns the MIME type of the image or an empty string if no image is loaded. - * @returns int + * @return int */ public function mimeType() { return $this->valid() ? $this->mimeType : ''; @@ -86,7 +87,7 @@ class OC_Image { /** * @brief Returns the width of the image or -1 if no image is loaded. - * @returns int + * @return int */ public function width() { return $this->valid() ? imagesx($this->resource) : -1; @@ -94,7 +95,7 @@ class OC_Image { /** * @brief Returns the height of the image or -1 if no image is loaded. - * @returns int + * @return int */ public function height() { return $this->valid() ? imagesy($this->resource) : -1; @@ -102,7 +103,7 @@ class OC_Image { /** * @brief Returns the width when the image orientation is top-left. - * @returns int + * @return int */ public function widthTopLeft() { $o = $this->getOrientation(); @@ -125,7 +126,7 @@ class OC_Image { /** * @brief Returns the height when the image orientation is top-left. - * @returns int + * @return int */ public function heightTopLeft() { $o = $this->getOrientation(); @@ -147,9 +148,10 @@ class OC_Image { } /** - * @brief Outputs the image. - * @returns bool - */ + * @brief Outputs the image. + * @param string $mimeType + * @return bool + */ public function show($mimeType=null) { if($mimeType === null) { $mimeType = $this->mimeType(); @@ -159,10 +161,11 @@ class OC_Image { } /** - * @brief Saves the image. - * @returns bool - * @param string $filePath - */ + * @brief Saves the image. + * @param string $filePath + * @param string $mimeType + * @return bool + */ public function save($filePath=null, $mimeType=null) { if($mimeType === null) { @@ -178,8 +181,12 @@ class OC_Image { } /** - * @brief Outputs/saves the image. - */ + * @brief Outputs/saves the image. + * @param string $filePath + * @param string $mimeType + * @return bool + * @throws Exception + */ private function _output($filePath=null, $mimeType=null) { if($filePath) { if (!file_exists(dirname($filePath))) @@ -198,7 +205,7 @@ class OC_Image { return false; } - $imageType = null; + $imageType = $this->imageType; if($mimeType !== null) { switch($mimeType) { case 'image/gif': @@ -218,10 +225,7 @@ class OC_Image { break; default: throw new Exception('\OC_Image::_output(): "' . $mimeType . '" is not supported when forcing a specific output format'); - break; } - } else { - $imageType = $this->imageType; } switch($imageType) { @@ -257,14 +261,14 @@ class OC_Image { } /** - * @returns resource Returns the image resource in any. + * @return resource Returns the image resource in any. */ public function resource() { return $this->resource; } /** - * @returns Returns the raw image data. + * @return string Returns the raw image data. */ function data() { ob_start(); @@ -299,7 +303,7 @@ class OC_Image { /** * (I'm open for suggestions on better method name ;) * @brief Get the orientation based on EXIF data. - * @returns The orientation or -1 if no EXIF data is available. + * @return int The orientation or -1 if no EXIF data is available. */ public function getOrientation() { if(!is_callable('exif_read_data')) { @@ -327,7 +331,7 @@ class OC_Image { /** * (I'm open for suggestions on better method name ;) * @brief Fixes orientation based on EXIF data. - * @returns bool. + * @return bool. */ public function fixOrientation() { $o = $this->getOrientation(); @@ -387,10 +391,10 @@ class OC_Image { } /** - * @brief Loads an image from a local file, a base64 encoded string or a resource created by an imagecreate* function. - * @param $imageref The path to a local file, a base64 encoded string or a resource created by an imagecreate* function or a file resource (file handle ). - * @returns An image resource or false on error - */ + * @brief Loads an image from a local file, a base64 encoded string or a resource created by an imagecreate* function. + * @param resource|string $imageref The path to a local file, a base64 encoded string or a resource created by an imagecreate* function or a file resource (file handle ). + * @return resource|false An image resource or false on error + */ public function load($imageRef) { if(is_resource($imageRef)) { if(get_resource_type($imageRef) == 'gd') { @@ -415,7 +419,7 @@ class OC_Image { * @brief Loads an image from an open file handle. * It is the responsibility of the caller to position the pointer at the correct place and to close the handle again. * @param resource $handle - * @returns An image resource or false on error + * @return An image resource or false on error */ public function loadFromFileHandle($handle) { OC_Log::write('core', __METHOD__.'(): Trying', OC_Log::DEBUG); @@ -427,8 +431,8 @@ class OC_Image { /** * @brief Loads an image from a local file. - * @param $imagePath The path to a local file. - * @returns An image resource or false on error + * @param bool|string $imagePath The path to a local file. + * @return bool|resource An image resource or false on error */ public function loadFromFile($imagePath=false) { // exif_imagetype throws "read error!" if file is less than 12 byte @@ -528,8 +532,8 @@ class OC_Image { /** * @brief Loads an image from a string of data. - * @param $str A string of image data as read from a file. - * @returns An image resource or false on error + * @param string $str A string of image data as read from a file. + * @return bool|resource An image resource or false on error */ public function loadFromData($str) { if(is_resource($str)) { @@ -553,8 +557,8 @@ class OC_Image { /** * @brief Loads an image from a base64 encoded string. - * @param $str A string base64 encoded string of image data. - * @returns An image resource or false on error + * @param string $str A string base64 encoded string of image data. + * @return bool|resource An image resource or false on error */ public function loadFromBase64($str) { if(!is_string($str)) { @@ -583,7 +587,7 @@ class OC_Image { * @param string $fileName

* Path to the BMP image. *

- * @return resource an image resource identifier on success, FALSE on errors. + * @return bool|resource an image resource identifier on success, FALSE on errors. */ private function imagecreatefrombmp($fileName) { if (!($fh = fopen($fileName, 'rb'))) { @@ -722,7 +726,7 @@ class OC_Image { /** * @brief Resizes the image preserving ratio. * @param integer $maxSize The maximum size of either the width or height. - * @returns bool + * @return bool */ public function resize($maxSize) { if(!$this->valid()) { @@ -745,6 +749,11 @@ class OC_Image { return true; } + /** + * @param int $width + * @param int $height + * @return bool + */ public function preciseResize($width, $height) { if (!$this->valid()) { OC_Log::write('core', __METHOD__.'(): No image loaded', OC_Log::ERROR); @@ -780,8 +789,8 @@ class OC_Image { /** * @brief Crops the image to the middle square. If the image is already square it just returns. - * @param int maximum size for the result (optional) - * @returns bool for success or failure + * @param int $size maximum size for the result (optional) + * @return bool for success or failure */ public function centerCrop($size=0) { if(!$this->valid()) { @@ -839,11 +848,11 @@ class OC_Image { /** * @brief Crops the image from point $x$y with dimension $wx$h. - * @param $x Horizontal position - * @param $y Vertical position - * @param $w Width - * @param $h Height - * @returns bool for success or failure + * @param int $x Horizontal position + * @param int $y Vertical position + * @param int $w Width + * @param int $h Height + * @return bool for success or failure */ public function crop($x, $y, $w, $h) { if(!$this->valid()) { @@ -871,7 +880,7 @@ class OC_Image { * @brief Resizes the image to fit within a boundry while preserving ratio. * @param integer $maxWidth * @param integer $maxHeight - * @returns bool + * @return bool */ public function fitIn($maxWidth, $maxHeight) { if(!$this->valid()) { From cd038604d396b7611353041cb8ed033f23fe305b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 17 Mar 2014 08:40:59 +0100 Subject: [PATCH 7/7] unit tests for specific image type output added --- lib/private/image.php | 7 ++++++- tests/lib/image.php | 24 ++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/private/image.php b/lib/private/image.php index dd041442ed6..c987ce92c3c 100644 --- a/lib/private/image.php +++ b/lib/private/image.php @@ -239,7 +239,12 @@ class OC_Image { $retVal = imagepng($this->resource, $filePath); break; case IMAGETYPE_XBM: - $retVal = imagexbm($this->resource, $filePath); + if (function_exists('imagexbm')) { + $retVal = imagexbm($this->resource, $filePath); + } else { + throw new Exception('\OC_Image::_output(): imagexbm() is not supported.'); + } + break; case IMAGETYPE_WBMP: $retVal = imagewbmp($this->resource, $filePath); diff --git a/tests/lib/image.php b/tests/lib/image.php index 4aba1b0bc61..131a9d86f3e 100644 --- a/tests/lib/image.php +++ b/tests/lib/image.php @@ -8,8 +8,8 @@ class Test_Image extends PHPUnit_Framework_TestCase { public static function tearDownAfterClass() { - unlink(OC::$SERVERROOT.'/tests/data/testimage2.png'); - unlink(OC::$SERVERROOT.'/tests/data/testimage2.jpg'); + @unlink(OC::$SERVERROOT.'/tests/data/testimage2.png'); + @unlink(OC::$SERVERROOT.'/tests/data/testimage2.jpg'); } public function testGetMimeTypeForFile() { @@ -236,4 +236,24 @@ class Test_Image extends PHPUnit_Framework_TestCase { $this->assertEquals(200, $img->width()); $this->assertEquals(200, $img->height()); } + + function convertDataProvider() { + return array( + array( 'image/gif'), + array( 'image/jpeg'), + array( 'image/png'), + ); + } + + /** + * @dataProvider convertDataProvider + */ + public function testConvert($mimeType) { + $img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png'); + $tempFile = tempnam(sys_get_temp_dir(), 'img-test'); + + $img->save($tempFile, $mimeType); + $actualMimeType = \OC_Image::getMimeTypeForFile($tempFile); + $this->assertEquals($mimeType, $actualMimeType); + } }