Improve OC_Image code to not guess the type of input, but actually request the specific methods to be called

Followup to #7836

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

core/Controller/AvatarController.php

@@ -285,7 +285,8 @@ class AvatarController extends Controller {
 									Http::STATUS_NOT_FOUND);
 		}
 
-		$image = new \OC_Image($tmpAvatar);
+		$image = new \OC_Image();
+		$image->loadFromData($tmpAvatar);
 
 		$resp = new DataDisplayResponse($image->data(),
 				Http::STATUS_OK,
@@ -322,7 +323,8 @@ class AvatarController extends Controller {
 									Http::STATUS_BAD_REQUEST);
 		}
 
-		$image = new \OC_Image($tmpAvatar);
+		$image = new \OC_Image();
+		$image->loadFromData($tmpAvatar);
 		$image->crop($crop['x'], $crop['y'], (int)round($crop['w']), (int)round($crop['h']));
 		try {
 			$avatar = $this->avatarManager->getAvatar($this->userId);

lib/private/Avatar.php

@@ -117,7 +117,24 @@ class Avatar implements IAvatar {
 			$img = $data;
 			$data = $img->data();
 		} else {
-			$img = new OC_Image($data);
+			$img = new OC_Image();
+			if (is_resource($data) && get_resource_type($data) === "gd") {
+				$img->setResource($data);
+			} elseif(is_resource($data)) {
+				$img->loadFromFileHandle($data);
+			} else {
+				try {
+					// detect if it is a path or maybe the images as string
+					$result = @realpath($data);
+					if ($result === false || $result === null) {
+						$img->loadFromData($data);
+					} else {
+						$img->loadFromFile($data);
+					}
+				} catch (\Error $e) {
+					$img->loadFromData($data);
+				}
+			}
 		}
 		$type = substr($img->mimeType(), -3);
 		if ($type === 'peg') {

lib/private/Preview/TXT.php

@@ -91,8 +91,9 @@ class TXT extends Provider {
 			}
 		}
 
-		$image = new \OC_Image($image);
+		$imageObject = new \OC_Image();
+		$imageObject->setResource($image);
 
-		return $image->valid() ? $image : false;
+		return $imageObject->valid() ? $imageObject : false;
 	}
 }

lib/private/PreviewManager.php

@@ -166,7 +166,9 @@ class PreviewManager implements IPreview {
 			return new \OC_Image();
 		}
 
-		return new \OC_Image($preview->getContent());
+		$previewImage = new \OC_Image();
+		$previewImage->loadFromData($preview->getContent());
+		return $previewImage;
 	}
 
 	/**

lib/private/legacy/image.php

@@ -69,6 +69,7 @@ class OC_Image implements \OCP\IImage {
 	 * an imagecreate* function.
 	 * @param \OCP\ILogger $logger
 	 * @param \OCP\IConfig $config
+	 * @throws \InvalidArgumentException in case the $imageRef parameter is not null
 	 */
 	public function __construct($imageRef = null, \OCP\ILogger $logger = null, \OCP\IConfig $config = null) {
 		$this->logger = $logger;
@@ -85,7 +86,7 @@ class OC_Image implements \OCP\IImage {
 		}
 
 		if ($imageRef !== null) {
-			$this->load($imageRef);
+			throw new \InvalidArgumentException('The first parameter in the constructor is not supported anymore. Please use any of the load* methods of the image object to load an image.');
 		}
 	}
 
@@ -297,6 +298,18 @@ class OC_Image implements \OCP\IImage {
 		return $this->show();
 	}
 
+	/**
+	 * @param resource Returns the image resource in any.
+	 * @throws \InvalidArgumentException in case the supplied resource does not have the type "gd"
+	 */
+	public function setResource($resource) {
+		if (get_resource_type($resource) === 'gd') {
+			$this->resource = $resource;
+			return;
+		}
+		throw new \InvalidArgumentException('Supplied resource is not of type "gd".');
+	}
+
 	/**
 	 * @return resource Returns the image resource in any.
 	 */
@@ -503,31 +516,6 @@ class OC_Image implements \OCP\IImage {
 		return false;
 	}
 
-	/**
-	 * 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') {
-				$this->resource = $imageRef;
-				return $this->resource;
-			} elseif (in_array(get_resource_type($imageRef), array('file', 'stream'))) {
-				return $this->loadFromFileHandle($imageRef);
-			}
-		} elseif ($this->loadFromBase64($imageRef) !== false) {
-			return $this->resource;
-		} elseif ($this->loadFromFile($imageRef) !== false) {
-			return $this->resource;
-		} elseif ($this->loadFromData($imageRef) !== false) {
-			return $this->resource;
-		}
-		$this->logger->debug(__METHOD__ . '(): could not load anything. Giving up!', array('app' => 'core'));
-		return false;
-	}
-
 	/**
 	 * 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.
@@ -550,16 +538,6 @@ class OC_Image implements \OCP\IImage {
 	 * @return bool|resource An image resource or false on error
 	 */
 	public function loadFromFile($imagePath = false) {
-		try {
-			// detect if it is a path or maybe the images as string
-			// needed because the constructor iterates over all load* methods
-			$result = @realpath($imagePath);
-			if ($result === false) {
-				return false;
-			}
-		} catch (Error $e) {
-			return false;
-		}
 		// exif_imagetype throws "read error!" if file is less than 12 byte
 		if (!@is_file($imagePath) || !file_exists($imagePath) || filesize($imagePath) < 12 || !is_readable($imagePath)) {
 			return false;

tests/lib/AvatarTest.php

@@ -89,7 +89,8 @@ class AvatarTest extends \Test\TestCase {
 				['avatar.128.jpg', true],
 			]));
 
-		$expected = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected = new \OC_Image();
+		$expected->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 
 		$file = $this->createMock(File::class);
 		$file->method('getContent')->willReturn($expected->data());
@@ -104,7 +105,8 @@ class AvatarTest extends \Test\TestCase {
 				['avatar.jpg', true],
 			]));
 
-		$expected = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected = new \OC_Image();
+		$expected->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 
 		$file = $this->createMock(File::class);
 		$file->method('getContent')->willReturn($expected->data());
@@ -120,8 +122,10 @@ class AvatarTest extends \Test\TestCase {
 				['avatar.32.png', false],
 			]));
 
-		$expected = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected = new \OC_Image();
-		$expected2 = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected2 = new \OC_Image();
+		$expected2->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 		$expected2->resize(32);
 
 		$file = $this->createMock(File::class);
@@ -205,7 +209,8 @@ class AvatarTest extends \Test\TestCase {
 			->with('avatar.png')
 			->willReturn($newFile);
 
-		$image = new \OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$image = new \OC_Image();
+		$image->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 		$newFile->expects($this->once())
 			->method('putContent')
 			->with($image->data());

tests/lib/ImageTest.php

@@ -20,97 +20,114 @@ class ImageTest extends \Test\TestCase {
 	}
 
 	public function testConstructDestruct() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertInstanceOf('\OC_Image', $img);
 		$this->assertInstanceOf('\OCP\IImage', $img);
 		unset($img);
 
 		$imgcreate = imagecreatefromjpeg(OC::$SERVERROOT.'/tests/data/testimage.jpg');
-		$img = new \OC_Image($imgcreate);
+		$img = new \OC_Image();
+		$img->setResource($imgcreate);
 		$this->assertInstanceOf('\OC_Image', $img);
 		$this->assertInstanceOf('\OCP\IImage', $img);
 		unset($img);
 
 		$base64 = base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif'));
-		$img = new \OC_Image($base64);
+		$img = new \OC_Image();
+		$img->loadFromBase64($base64);
 		$this->assertInstanceOf('\OC_Image', $img);
 		$this->assertInstanceOf('\OCP\IImage', $img);
 		unset($img);
 
-		$img = new \OC_Image(null);
+		$img = new \OC_Image();
 		$this->assertInstanceOf('\OC_Image', $img);
 		$this->assertInstanceOf('\OCP\IImage', $img);
 		unset($img);
 	}
 
 	public function testValid() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertTrue($img->valid());
 
 		$text = base64_encode("Lorem ipsum dolor sir amet …");
-		$img = new \OC_Image($text);
+		$img = new \OC_Image();
+		$img->loadFromBase64($text);
 		$this->assertFalse($img->valid());
 
-		$img = new \OC_Image(null);
+		$img = new \OC_Image();
 		$this->assertFalse($img->valid());
 	}
 
 	public function testMimeType() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertEquals('image/png', $img->mimeType());
 
-		$img = new \OC_Image(null);
+		$img = new \OC_Image();
 		$this->assertEquals('', $img->mimeType());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$this->assertEquals('image/jpeg', $img->mimeType());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$this->assertEquals('image/gif', $img->mimeType());
 	}
 
 	public function testWidth() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertEquals(128, $img->width());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$this->assertEquals(1680, $img->width());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$this->assertEquals(64, $img->width());
 
-		$img = new \OC_Image(null);
+		$img = new \OC_Image();
 		$this->assertEquals(-1, $img->width());
 	}
 
 	public function testHeight() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertEquals(128, $img->height());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$this->assertEquals(1050, $img->height());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$this->assertEquals(64, $img->height());
 
-		$img = new \OC_Image(null);
+		$img = new \OC_Image();
 		$this->assertEquals(-1, $img->height());
 	}
 
 	public function testSave() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$img->resize(16);
 		$img->save(OC::$SERVERROOT.'/tests/data/testimage2.png');
 		$this->assertEquals(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage2.png'), $img->data());
 
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.jpg');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.jpg');
 		$img->resize(128);
 		$img->save(OC::$SERVERROOT.'/tests/data/testimage2.jpg');
 		$this->assertEquals(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage2.jpg'), $img->data());
 	}
 
 	public function testData() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$raw = imagecreatefromstring(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.png'));
 		// Preserve transparency
 		imagealphablending($raw, true);
@@ -125,14 +142,16 @@ class ImageTest extends \Test\TestCase {
 			->method('getAppValue')
 			->with('preview', 'jpeg_quality', 90)
 			->willReturn(null);
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.jpg', null, $config);
+		$img = new \OC_Image(null, null, $config);
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.jpg');
 		$raw = imagecreatefromstring(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		ob_start();
 		imagejpeg($raw);
 		$expected = ob_get_clean();
 		$this->assertEquals($expected, $img->data());
 
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.gif');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.gif');
 		$raw = imagecreatefromstring(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif'));
 		ob_start();
 		imagegif($raw);
@@ -149,82 +168,97 @@ class ImageTest extends \Test\TestCase {
 	 * @depends testData
 	 */
 	public function testToString() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$expected = base64_encode($img->data());
 		$this->assertEquals($expected, (string)$img);
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$expected = base64_encode($img->data());
 		$this->assertEquals($expected, (string)$img);
 
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.gif');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.gif');
 		$expected = base64_encode($img->data());
 		$this->assertEquals($expected, (string)$img);
 	}
 
 	public function testResize() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertTrue($img->resize(32));
 		$this->assertEquals(32, $img->width());
 		$this->assertEquals(32, $img->height());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$this->assertTrue($img->resize(840));
 		$this->assertEquals(840, $img->width());
 		$this->assertEquals(525, $img->height());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$this->assertTrue($img->resize(100));
 		$this->assertEquals(100, $img->width());
 		$this->assertEquals(100, $img->height());
 	}
 
 	public function testPreciseResize() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertTrue($img->preciseResize(128, 512));
 		$this->assertEquals(128, $img->width());
 		$this->assertEquals(512, $img->height());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$this->assertTrue($img->preciseResize(64, 840));
 		$this->assertEquals(64, $img->width());
 		$this->assertEquals(840, $img->height());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$this->assertTrue($img->preciseResize(1000, 1337));
 		$this->assertEquals(1000, $img->width());
 		$this->assertEquals(1337, $img->height());
 	}
 
 	public function testCenterCrop() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$img->centerCrop();
 		$this->assertEquals(128, $img->width());
 		$this->assertEquals(128, $img->height());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$img->centerCrop();
 		$this->assertEquals(1050, $img->width());
 		$this->assertEquals(1050, $img->height());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$img->centerCrop(512);
 		$this->assertEquals(512, $img->width());
 		$this->assertEquals(512, $img->height());
 	}
 
 	public function testCrop() {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$this->assertTrue($img->crop(0, 0, 50, 20));
 		$this->assertEquals(50, $img->width());
 		$this->assertEquals(20, $img->height());
 
-		$img = new \OC_Image(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
+		$img = new \OC_Image();
+		$img->loadFromData(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
 		$this->assertTrue($img->crop(500, 700, 550, 300));
 		$this->assertEquals(550, $img->width());
 		$this->assertEquals(300, $img->height());
 
-		$img = new \OC_Image(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
+		$img = new \OC_Image();
+		$img->loadFromBase64(base64_encode(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.gif')));
 		$this->assertTrue($img->crop(10, 10, 15, 15));
 		$this->assertEquals(15, $img->width());
 		$this->assertEquals(15, $img->height());
@@ -246,7 +280,8 @@ class ImageTest extends \Test\TestCase {
 	 * @param int[] $expected
 	 */
 	public function testFitIn($filename, $asked, $expected) {
-		$img = new \OC_Image(OC::$SERVERROOT . '/tests/data/' . $filename);
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT . '/tests/data/' . $filename);
 		$this->assertTrue($img->fitIn($asked[0], $asked[1]));
 		$this->assertEquals($expected[0], $img->width());
 		$this->assertEquals($expected[1], $img->height());
@@ -268,7 +303,8 @@ class ImageTest extends \Test\TestCase {
 	 * @param string $filename
 	 */
 	public function testScaleDownToFitWhenSmallerAlready($filename) {
-		$img = new \OC_Image(OC::$SERVERROOT . '/tests/data/' . $filename);
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/' . $filename);
 		$currentWidth = $img->width();
 		$currentHeight = $img->height();
 		// We pick something larger than the image we want to scale down
@@ -300,7 +336,8 @@ class ImageTest extends \Test\TestCase {
 	 * @param int[] $expected
 	 */
 	public function testScaleDownWhenBigger($filename, $asked, $expected) {
-		$img = new \OC_Image(OC::$SERVERROOT . '/tests/data/' . $filename);
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/' . $filename);
 		//$this->assertTrue($img->scaleDownToFit($asked[0], $asked[1]));
 		$img->scaleDownToFit($asked[0], $asked[1]);
 		$this->assertEquals($expected[0], $img->width());
@@ -319,7 +356,8 @@ class ImageTest extends \Test\TestCase {
 	 * @dataProvider convertDataProvider
 	 */
 	public function testConvert($mimeType) {
-		$img = new \OC_Image(OC::$SERVERROOT.'/tests/data/testimage.png');
+		$img = new \OC_Image();
+		$img->loadFromFile(OC::$SERVERROOT.'/tests/data/testimage.png');
 		$tempFile = tempnam(sys_get_temp_dir(), 'img-test');
 
 		$img->save($tempFile, $mimeType);