Merge pull request #465 from christeredvartsen/issue-462

Respond with 404 when trying to add a short URL to an image that does not exist. Close #462.
imbo · Apr 14, 2016 · d2e937a · d2e937a
2 parents 992b2e5 + 263da62
commit d2e937a
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 11 deletions.
diff --git a/ChangeLog.markdown b/ChangeLog.markdown
@@ -5,10 +5,14 @@ Imbo-x.x.x
 ----------
 __N/A__
 
-* #463: Fixed issue with an error model being formatted as an image (Christer Edvartsen)
 * #444: Added a getData() method to the Imbo\Model\ModelInterface (Christer Edvartsen)
 * #431: Added an Amazon S3 storage adapter for the image variations (Ali Asaria)
 
+Bug fixes:
+
+* #463: Fixed issue with an error model being formatted as an image (Christer Edvartsen)
+* #462: Adding short URLs to an image that does not exist now results in 404 (Christer Edvartsen)
+
 Imbo-2.1.1
 ----------
 __2016-03-11__
@@ -26,29 +30,32 @@ Imbo-2.0.0
 ----------
 __2016-02-28__
 
-* #430: Prevent race conditions in image transformation cache (Espen Hovlandsdal)
 * #429: Added opacity to watermark image (Sindre Gulseth)
 * #427: When resizing with one param round up the other calculated value (Sindre Gulseth)
 * #423: Make storage drivers throw exceptions as expected in StorageOperations (Mats Lindh)
 * #410: Validate incoming image metadata (Kristoffer Brabrand)
-* #402: Fix Strip-transformation not doing anything on newer Imagick versions (Espen Hovlandsdal)
 * #402: Improve Contrast-transformation predictability (Espen Hovlandsdal)
 * #400: New image transformation: Blur (Kristoffer Brabrand)
 * #398: Add ability to configure HTTP cache headers (Espen Hovlandsdal)
 * #391: Make Crop-transformation validate coordinates (Espen Hovlandsdal)
-* #390: Fix image variation + crop transformation bug (Espen Hovlandsdal)
-* #386: Fix CORS wildcard-issue when client did not send `Origin`-header (Espen Hovlandsdal)
 * #381: New image transformation: DrawPois (points of interest) (Espen Hovlandsdal)
 * #376: Add config option to alter protocol used for authentication signatures (Espen Hovlandsdal)
-* #367: Fix bug where special characters could break metadata XML response (Kristoffer Brabrand)
-* #366: Fix border transformation + alpha channel bug (Espen Hovlandsdal)
 * #363: Add pluggable image identifier generation (Espen Hovlandsdal)
 * #357: Add public key generation CLI-command (Espen Hovlandsdal)
 * #351: New image transformation: SmartSize (Kristoffer Brabrand, Espen Hovlandsdal)
 * #348: Add global images endpoint (Kristoffer Brabrand)
 * #347: Use UUID instead of MD5 for image identifiers (Espen Hovlandsdal)
 * #328: New access control implementation (Espen Hovlandsdal, Kristoffer Brabrand)
 
+Bug fixes:
+
+* #430: Prevent race conditions in image transformation cache (Espen Hovlandsdal)
+* #402: Fix Strip-transformation not doing anything on newer Imagick versions (Espen Hovlandsdal)
+* #390: Fix image variation + crop transformation bug (Espen Hovlandsdal)
+* #386: Fix CORS wildcard-issue when client did not send `Origin`-header (Espen Hovlandsdal)
+* #367: Fix bug where special characters could break metadata XML response (Kristoffer Brabrand)
+* #366: Fix border transformation + alpha channel bug (Espen Hovlandsdal)
+
 Imbo-1.2.5
 ----------
 __2015-08-14__

diff --git a/library/Imbo/Resource/ShortUrls.php b/library/Imbo/Resource/ShortUrls.php
@@ -86,6 +86,10 @@ public function createShortUrl(EventInterface $event) {
 
         $database = $event->getDatabase();
 
+        if (!$database->imageExists($image['user'], $image['imageIdentifier'])) {
+            throw new InvalidArgumentException('Image does not exist', 404);
+        }
+
         // See if a short URL ID already exists the for given parameters
         $exists = true;
         $shortUrlId = $database->getShortUrlId($image['user'], $image['imageIdentifier'], $extension, $query);

diff --git a/tests/behat/features/shorturls.feature b/tests/behat/features/shorturls.feature
@@ -3,6 +3,21 @@ Feature: Imbo can generate short URLs for images on demand
     As an HTTP Client
     I can request the short URLs resource
 
+    Scenario: Responds with 404 when the image does not exist
+        Given I use "publickey" and "privatekey" for public and private keys
+        And I sign the request
+        And the request body contains:
+            """
+            {"user": "user", "imageIdentifier": "id", "extension": "gif", "query": null}
+            """
+        When I request "/users/user/images/id/shorturls" using HTTP "POST"
+        Then I should get a response with "404 Image does not exist"
+        And the "Content-Type" response header is "application/json"
+        And the response body matches:
+           """
+           #^{"error":{"code":404,"message":"Image does not exist".*?,"imageIdentifier":"id"}$#
+           """
+
     Scenario: Generate a short URL
         Given "tests/phpunit/Fixtures/image.png" exists in Imbo
         And I use "publickey" and "privatekey" for public and private keys

diff --git a/tests/phpunit/ImboUnitTest/Resource/ShortUrlsTest.php b/tests/phpunit/ImboUnitTest/Resource/ShortUrlsTest.php
@@ -178,6 +178,7 @@ public function testCanCreateShortUrls($extension = null, $queryString = null, a
         '));
         $this->request->expects($this->once())->method('getUser')->will($this->returnValue('user'));
         $this->request->expects($this->once())->method('getImageIdentifier')->will($this->returnValue('id'));
+        $this->database->expects($this->once())->method('imageExists')->with('user', 'id')->will($this->returnValue(true));
         $this->database->expects($this->once())->method('getShortUrlId')->with('user', 'id', $extension, $query)->will($this->returnValue(null));
         $this->database->expects($this->once())->method('getShortUrlParams')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'))->will($this->returnValue(null));
         $this->database->expects($this->once())->method('insertShortUrl')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'), 'user', 'id', $extension, $query);
@@ -198,6 +199,7 @@ public function testWillReturn200OKIfTheShortUrlAlreadyExists() {
         '));
         $this->request->expects($this->once())->method('getUser')->will($this->returnValue('user'));
         $this->request->expects($this->once())->method('getImageIdentifier')->will($this->returnValue('id'));
+        $this->database->expects($this->once())->method('imageExists')->with('user', 'id')->will($this->returnValue(true));
         $this->database->expects($this->once())->method('getShortUrlId')->with('user', 'id', null, [])->will($this->returnValue('aaaaaaa'));
         $this->database->expects($this->never())->method('insertShortUrl');
         $this->response->expects($this->once())->method('setModel')->with($this->isInstanceOf('Imbo\Model\ArrayModel'))->will($this->returnSelf());
@@ -211,11 +213,12 @@ public function testWillGenerateANewIdIfTheGeneratedOneExists() {
         $this->request->expects($this->once())->method('getUser')->will($this->returnValue('user'));
         $this->request->expects($this->once())->method('getImageIdentifier')->will($this->returnValue('id'));
 
-        $this->database->expects($this->at(0))->method('getShortUrlId')->with('user', 'id', null, [])->will($this->returnValue(null));
-        $this->database->expects($this->at(1))->method('getShortUrlParams')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'))->will($this->returnValue(['user' => 'value']));
+        $this->database->expects($this->at(0))->method('imageExists')->with('user', 'id')->will($this->returnValue(true));
+        $this->database->expects($this->at(1))->method('getShortUrlId')->with('user', 'id', null, [])->will($this->returnValue(null));
         $this->database->expects($this->at(2))->method('getShortUrlParams')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'))->will($this->returnValue(['user' => 'value']));
-        $this->database->expects($this->at(3))->method('getShortUrlParams')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'))->will($this->returnValue(null));
-        $this->database->expects($this->at(4))->method('insertShortUrl')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'), 'user', 'id', null, []);
+        $this->database->expects($this->at(3))->method('getShortUrlParams')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'))->will($this->returnValue(['user' => 'value']));
+        $this->database->expects($this->at(4))->method('getShortUrlParams')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'))->will($this->returnValue(null));
+        $this->database->expects($this->at(5))->method('insertShortUrl')->with($this->matchesRegularExpression('/[a-zA-Z0-9]{7}/'), 'user', 'id', null, []);
 
         $this->response->expects($this->once())->method('setModel')->with($this->isInstanceOf('Imbo\Model\ArrayModel'))->will($this->returnSelf());
         $this->response->expects($this->once())->method('setStatusCode')->with(201);
@@ -242,4 +245,25 @@ public function testWillAddAModelIfTheEventIsAShortUrlsEvent() {
 
         $this->getNewResource()->deleteImageShortUrls($this->event);
     }
+
+    /**
+     * @expectedException InvalidArgumentException
+     * @expectedExceptionMessage Image does not exist
+     * @expectedExceptionCode 404
+     */
+    public function testCanNotAddShortUrlWhenImageDoesNotExist() {
+        $this->request->expects($this->once())->method('getContent')->will($this->returnValue('
+            {
+                "user": "user",
+                "imageIdentifier": "id",
+                "extension": null,
+                "query": null
+            }
+        '));
+        $this->request->expects($this->once())->method('getUser')->will($this->returnValue('user'));
+        $this->request->expects($this->once())->method('getImageIdentifier')->will($this->returnValue('id'));
+        $this->database->expects($this->once())->method('imageExists')->with('user', 'id')->will($this->returnValue(false));
+
+        $this->getNewResource()->createShortUrl($this->event);
+    }
 }