From a4698de75c7a38f9fa9d07695fc2aaad519ac8e3 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 4 May 2020 04:04:12 +0200 Subject: [PATCH 01/15] helpers\Image: clean up a bit --- src/helpers/ContentLoader.php | 10 ++++++---- src/helpers/Image.php | 35 +++++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index 9720f41dea..c879b13646 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -317,8 +317,9 @@ protected function sanitizeField($value) { */ protected function fetchThumbnail($thumbnail, array $newItem) { if (strlen(trim($thumbnail)) > 0) { - $extension = 'jpg'; - $thumbnailAsJpg = $this->imageHelper->loadImage($thumbnail, $extension, 500, 500); + $format = Image::FORMAT_JPEG; + $extension = Image::getExtension($format); + $thumbnailAsJpg = $this->imageHelper->loadImage($thumbnail, $format, 500, 500); if ($thumbnailAsJpg !== null) { $written = file_put_contents( \F3::get('datadir') . '/thumbnails/' . md5($thumbnail) . '.' . $extension, @@ -350,12 +351,13 @@ protected function fetchThumbnail($thumbnail, array $newItem) { */ protected function fetchIcon($icon, array $newItem, &$lasticon) { if (strlen(trim($icon)) > 0) { - $extension = 'png'; + $format = Image::FORMAT_PNG; + $extension = Image::getExtension($format); if ($icon === $lasticon) { $this->logger->debug('use last icon: ' . $lasticon); $newItem['icon'] = md5($lasticon) . '.' . $extension; } else { - $iconAsPng = $this->imageHelper->loadImage($icon, $extension, 30, null); + $iconAsPng = $this->imageHelper->loadImage($icon, $format, 30, null); if ($iconAsPng !== null) { $written = file_put_contents( \F3::get('datadir') . '/favicons/' . md5($icon) . '.' . $extension, diff --git a/src/helpers/Image.php b/src/helpers/Image.php index 8fe1bcf3db..d6635f21da 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -16,9 +16,17 @@ * @author Tobias Zeising */ class Image { + const FORMAT_JPEG = 'jpeg'; + const FORMAT_PNG = 'png'; + /** @var ?string url of last fetched favicon */ private $faviconUrl = null; + private static $extensions = [ + self::FORMAT_JPEG => 'jpg', + self::FORMAT_PNG => 'png', + ]; + private static $imageTypes = [ // IANA assigned type 'image/bmp' => 'bmp', @@ -56,6 +64,17 @@ public function __construct(Logger $logger, WebClient $webClient) { $this->webClient = $webClient; } + /** + * Get preferred extension for the format. + * + * @param self::FORMAT_* $format + * + * @return string + */ + public static function getExtension($format) { + return self::$extensions[$format]; + } + /** * fetch favicon * @@ -123,16 +142,16 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = } /** - * load image + * Load image from URL, optionally resize it and convert it to desired format. * * @param string $url source url - * @param string $extension file extension of output file - * @param ?int $width - * @param ?int $height + * @param self::FORMAT_JPEG|self::FORMAT_PNG $format file format of output file + * @param ?int $width target width + * @param ?int $height target height * - * @return ?string + * @return ?string blob containing the processed image */ - public function loadImage($url, $extension = 'png', $width = null, $height = null) { + public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $height = null) { // load image try { $data = $this->webClient->request($url); @@ -176,7 +195,7 @@ public function loadImage($url, $extension = 'png', $width = null, $height = nul } $image->resizeImage($width, $height, \Imagick::FILTER_LANCZOS, 1); - if ($extension === 'jpg') { + if ($format === self::FORMAT_JPEG) { $image->setImageFormat('jpeg'); $image->setImageCompression(\Imagick::COMPRESSION_JPEG); $image->setImageCompressionQuality(75); @@ -237,7 +256,7 @@ public function loadImage($url, $extension = 'png', $width = null, $height = nul } // return image as jpg or png - if ($extension === 'jpg') { + if ($format === self::FORMAT_JPEG) { $data = $wideImage->asString('jpg', 75); } else { $data = $wideImage->asString('png', 4, PNG_NO_FILTER); From 15792dbaf727b965c8a1f03f6d3853e7e6179f02 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 4 May 2020 05:29:20 +0200 Subject: [PATCH 02/15] helpers\Image: Remove favicon state --- src/helpers/Image.php | 26 ++++---------------------- src/spouts/reddit/reddit2.php | 4 ++-- src/spouts/rss/feed.php | 12 ++++++------ 3 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/helpers/Image.php b/src/helpers/Image.php index d6635f21da..f810aedcb7 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -19,9 +19,6 @@ class Image { const FORMAT_JPEG = 'jpeg'; const FORMAT_PNG = 'png'; - /** @var ?string url of last fetched favicon */ - private $faviconUrl = null; - private static $extensions = [ self::FORMAT_JPEG => 'jpg', self::FORMAT_PNG => 'png', @@ -83,16 +80,14 @@ public static function getExtension($format) { * @param ?int $width * @param ?int $height * - * @return ?string + * @return ?array{string, string} pair of URL and blob containing the image data */ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = null) { // try given url if ($isHtmlUrl === false) { $faviconAsPng = $this->loadImage($url, $width, $height); if ($faviconAsPng !== null) { - $this->faviconUrl = $url; - - return $faviconAsPng; + return [$url, $faviconAsPng]; } } @@ -120,9 +115,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = $faviconAsPng = $this->loadImage($shortcutIconUrl, $width, $height); if ($faviconAsPng !== null) { - $this->faviconUrl = $shortcutIconUrl; - - return $faviconAsPng; + return [$shortcutIconUrl, $faviconAsPng]; } } } @@ -132,9 +125,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = $url = $urlElements['scheme'] . '://' . $urlElements['host'] . '/favicon.ico'; $faviconAsPng = $this->loadImage($url, $width, $height); if ($faviconAsPng !== null) { - $this->faviconUrl = $url; - - return $faviconAsPng; + return [$url, $faviconAsPng]; } } @@ -265,15 +256,6 @@ public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $heig return $data; } - /** - * get favicon url - * - * @return ?string - */ - public function getFaviconUrl() { - return $this->faviconUrl; - } - /** * removes $startString, $endString and everything in between from $subject * diff --git a/src/spouts/reddit/reddit2.php b/src/spouts/reddit/reddit2.php index 8b6ac4ce83..75287242f6 100644 --- a/src/spouts/reddit/reddit2.php +++ b/src/spouts/reddit/reddit2.php @@ -158,8 +158,8 @@ public function getContent() { public function getIcon() { $htmlUrl = $this->getHtmlUrl(); - if ($htmlUrl && $this->imageHelper->fetchFavicon($htmlUrl)) { - $this->faviconUrl = $this->imageHelper->getFaviconUrl(); + if ($htmlUrl && ($iconData = $this->imageHelper->fetchFavicon($htmlUrl)) !== null) { + list($this->faviconUrl, $iconBlob) = $iconData; } return $this->faviconUrl; diff --git a/src/spouts/rss/feed.php b/src/spouts/rss/feed.php index b3c4a08ff8..2fe0941d62 100644 --- a/src/spouts/rss/feed.php +++ b/src/spouts/rss/feed.php @@ -109,8 +109,8 @@ public function getIcon() { // Try to use feed logo first $feedLogoUrl = $this->feed->getImageUrl(); - if ($feedLogoUrl && $this->imageHelper->fetchFavicon($feedLogoUrl)) { - $this->faviconUrl = $this->imageHelper->getFaviconUrl(); + if ($feedLogoUrl && ($iconData = $this->imageHelper->fetchFavicon($feedLogoUrl)) !== null) { + list($this->faviconUrl, $iconBlob) = $iconData; $this->logger->debug('icon: using feed logo: ' . $this->faviconUrl); return $this->faviconUrl; @@ -118,8 +118,8 @@ public function getIcon() { // else fallback to the favicon of the associated web page $htmlUrl = $this->getHtmlUrl(); - if ($htmlUrl && $this->imageHelper->fetchFavicon($htmlUrl, true)) { - $this->faviconUrl = $this->imageHelper->getFaviconUrl(); + if ($htmlUrl && ($iconData = $this->imageHelper->fetchFavicon($htmlUrl, true)) !== null) { + list($this->faviconUrl, $iconBlob) = $iconData; $this->logger->debug('icon: using feed homepage favicon: ' . $this->faviconUrl); return $this->faviconUrl; @@ -127,8 +127,8 @@ public function getIcon() { // else fallback to the favicon of the feed effective domain $feedUrl = $this->feed->getFeedUrl(); - if ($feedUrl && $this->imageHelper->fetchFavicon($feedUrl, true)) { - $this->faviconUrl = $this->imageHelper->getFaviconUrl(); + if ($feedUrl && ($iconData = $this->imageHelper->fetchFavicon($feedUrl, true)) !== null) { + list($this->faviconUrl, $iconBlob) = $iconData; $this->logger->debug('icon: using feed homepage favicon: ' . $this->faviconUrl); return $this->faviconUrl; From 9413ea3b60f2a3be5b2fee69a09078b4b7b929a4 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 4 May 2020 05:30:25 +0200 Subject: [PATCH 03/15] helpers\Image: fix some typos Thanks Psalm for noticing. Weird that this has ever worked. It was broken for five years: https://github.com/SSilence/selfoss/pull/639 --- src/helpers/Image.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/helpers/Image.php b/src/helpers/Image.php index f810aedcb7..a07d6be8a1 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -85,7 +85,7 @@ public static function getExtension($format) { public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = null) { // try given url if ($isHtmlUrl === false) { - $faviconAsPng = $this->loadImage($url, $width, $height); + $faviconAsPng = $this->loadImage($url, self::FORMAT_PNG, $width, $height); if ($faviconAsPng !== null) { return [$url, $faviconAsPng]; } @@ -102,7 +102,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = $effectiveUrl = new Uri(WebClient::getEffectiveUrl($url, $response)); if ($response->getStatusCode() !== 200) { - throw new Exception(substr($html, 0, 512)); + throw new \Exception(substr($html, 0, 512)); } } catch (\Exception $e) { $this->logger->debug('icon: failed to get html page: ', ['exception' => $e]); @@ -113,7 +113,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = foreach ($shortcutIcons as $shortcutIcon) { $shortcutIconUrl = (string) UriResolver::resolve($effectiveUrl, new Uri($shortcutIcon)); - $faviconAsPng = $this->loadImage($shortcutIconUrl, $width, $height); + $faviconAsPng = $this->loadImage($shortcutIconUrl, self::FORMAT_PNG, $width, $height); if ($faviconAsPng !== null) { return [$shortcutIconUrl, $faviconAsPng]; } @@ -123,7 +123,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = // search domain/favicon.ico if (isset($urlElements['scheme']) && isset($urlElements['host'])) { $url = $urlElements['scheme'] . '://' . $urlElements['host'] . '/favicon.ico'; - $faviconAsPng = $this->loadImage($url, $width, $height); + $faviconAsPng = $this->loadImage($url, self::FORMAT_PNG, $width, $height); if ($faviconAsPng !== null) { return [$url, $faviconAsPng]; } From 4c1f1fa77f5f95d3e483df3bb494a305b65a0472 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 4 May 2020 23:22:37 +0200 Subject: [PATCH 04/15] helpers\ContentLoader: Simplify image fetchers No need to pass the item through. --- src/helpers/ContentLoader.php | 81 ++++++++++++++++------------------- 1 file changed, 37 insertions(+), 44 deletions(-) diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index c879b13646..d78f177c6b 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -192,31 +192,25 @@ public function fetch($source) { $this->logger->debug('item content sanitized'); - try { - $icon = $item->getIcon(); - } catch (\Exception $e) { - $this->logger->debug('icon: error', ['exception' => $e]); - - return; - } - $newItem = [ 'title' => $title, 'content' => $content, 'source' => $source['id'], 'datetime' => $itemDate->format('Y-m-d H:i:s'), 'uid' => $item->getId(), - 'thumbnail' => $item->getThumbnail(), - 'icon' => $icon !== null ? $icon : '', 'link' => htmLawed($item->getLink(), ['deny_attribute' => '*', 'elements' => '-*']), 'author' => $author ]; // save thumbnail - $newItem = $this->fetchThumbnail($item->getThumbnail(), $newItem); + $newItem['thumbnail'] = $this->fetchThumbnail($item->getThumbnail()) ?: ''; - // save icon - $newItem = $this->fetchIcon($item->getIcon(), $newItem, $lasticon); + try { + // save icon + $newItem['icon'] = $this->fetchIcon($item->getIcon(), $lasticon) ?: ''; + } catch (\Exception $e) { + $this->logger->error('icon: error', ['exception' => $e]); + } // insert new item $this->itemsDao->add($newItem); @@ -308,78 +302,77 @@ protected function sanitizeField($value) { } /** - * Fetch the thumbanil of a given item + * Fetch an image URL and process it as a thumbnail. * - * @param string $thumbnail the thumbnail url - * @param array $newItem new item for saving in database + * @param string $url the thumbnail URL * - * @return array the newItem Object with thumbnail + * @return ?string path in the thumbnails directory */ - protected function fetchThumbnail($thumbnail, array $newItem) { - if (strlen(trim($thumbnail)) > 0) { + protected function fetchThumbnail($url) { + if (strlen(trim($url)) > 0) { $format = Image::FORMAT_JPEG; $extension = Image::getExtension($format); - $thumbnailAsJpg = $this->imageHelper->loadImage($thumbnail, $format, 500, 500); + $thumbnailAsJpg = $this->imageHelper->loadImage($url, $format, 500, 500); if ($thumbnailAsJpg !== null) { $written = file_put_contents( - \F3::get('datadir') . '/thumbnails/' . md5($thumbnail) . '.' . $extension, + \F3::get('datadir') . '/thumbnails/' . md5($url) . '.' . $extension, $thumbnailAsJpg ); if ($written !== false) { - $newItem['thumbnail'] = md5($thumbnail) . '.' . $extension; - $this->logger->debug('Thumbnail generated: ' . $thumbnail); + $this->logger->debug('Thumbnail generated: ' . $url); + + return md5($url) . '.' . $extension; } else { - $this->logger->warning('Unable to store thumbnail: ' . $thumbnail . '. Please check permissions of ' . \F3::get('datadir') . '/thumbnails.'); + $this->logger->warning('Unable to store thumbnail: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/thumbnails.'); } } else { - $newItem['thumbnail'] = ''; - $this->logger->error('thumbnail generation error: ' . $thumbnail); + $this->logger->error('thumbnail generation error: ' . $url); } } - return $newItem; + return null; } /** - * Fetch the icon of a given feed item + * Fetch an image and process it as favicon. * - * @param string $icon icon given by the spout - * @param array $newItem new item for saving in database + * @param string $url icon given by the spout * @param &string $lasticon the last fetched icon * - * @return mixed newItem with icon + * @return ?string path in the favicons directory */ - protected function fetchIcon($icon, array $newItem, &$lasticon) { - if (strlen(trim($icon)) > 0) { + protected function fetchIcon($url, &$lasticon) { + if (strlen(trim($url)) > 0) { $format = Image::FORMAT_PNG; $extension = Image::getExtension($format); - if ($icon === $lasticon) { + if ($url === $lasticon) { $this->logger->debug('use last icon: ' . $lasticon); - $newItem['icon'] = md5($lasticon) . '.' . $extension; + + return md5($lasticon) . '.' . $extension; } else { - $iconAsPng = $this->imageHelper->loadImage($icon, $format, 30, null); + $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); if ($iconAsPng !== null) { $written = file_put_contents( - \F3::get('datadir') . '/favicons/' . md5($icon) . '.' . $extension, + \F3::get('datadir') . '/favicons/' . md5($url) . '.' . $extension, $iconAsPng ); - $lasticon = $icon; + $lasticon = $url; if ($written !== false) { - $newItem['icon'] = md5($icon) . '.' . $extension; - $this->logger->debug('Icon generated: ' . $icon); + $this->logger->debug('Icon generated: ' . $url); + + return md5($url) . '.' . $extension; } else { - $this->logger->warning('Unable to store icon: ' . $icon . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); + $this->logger->warning('Unable to store icon: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); } } else { - $newItem['icon'] = ''; - $this->logger->error('icon generation error: ' . $icon); + $this->logger->error('icon generation error: ' . $url); } } } else { $this->logger->debug('no icon for this feed'); } - return $newItem; + return null; } /** From 08485a9ebef680a38ce610a3a2826d46d6323240 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 4 May 2020 23:58:01 +0200 Subject: [PATCH 05/15] helpers\ContentLoader: Simplify image fetchers more --- src/helpers/ContentLoader.php | 90 ++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index d78f177c6b..6601beb1f1 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -202,12 +202,20 @@ public function fetch($source) { 'author' => $author ]; - // save thumbnail - $newItem['thumbnail'] = $this->fetchThumbnail($item->getThumbnail()) ?: ''; + $thumbnailUrl = $item->getThumbnail(); + if (strlen(trim($thumbnailUrl)) > 0) { + // save thumbnail + $newItem['thumbnail'] = $this->fetchThumbnail($thumbnailUrl) ?: ''; + } try { - // save icon - $newItem['icon'] = $this->fetchIcon($item->getIcon(), $lasticon) ?: ''; + $iconUrl = $item->getIcon(); + if (strlen(trim($iconUrl)) > 0) { + // save icon + $newItem['icon'] = $this->fetchIcon($iconUrl, $lasticon) ?: ''; + } else { + $this->logger->debug('no icon for this feed'); + } } catch (\Exception $e) { $this->logger->error('icon: error', ['exception' => $e]); } @@ -309,25 +317,23 @@ protected function sanitizeField($value) { * @return ?string path in the thumbnails directory */ protected function fetchThumbnail($url) { - if (strlen(trim($url)) > 0) { - $format = Image::FORMAT_JPEG; - $extension = Image::getExtension($format); - $thumbnailAsJpg = $this->imageHelper->loadImage($url, $format, 500, 500); - if ($thumbnailAsJpg !== null) { - $written = file_put_contents( - \F3::get('datadir') . '/thumbnails/' . md5($url) . '.' . $extension, - $thumbnailAsJpg - ); - if ($written !== false) { - $this->logger->debug('Thumbnail generated: ' . $url); + $format = Image::FORMAT_JPEG; + $extension = Image::getExtension($format); + $thumbnailAsJpg = $this->imageHelper->loadImage($url, $format, 500, 500); + if ($thumbnailAsJpg !== null) { + $written = file_put_contents( + \F3::get('datadir') . '/thumbnails/' . md5($url) . '.' . $extension, + $thumbnailAsJpg + ); + if ($written !== false) { + $this->logger->debug('Thumbnail generated: ' . $url); - return md5($url) . '.' . $extension; - } else { - $this->logger->warning('Unable to store thumbnail: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/thumbnails.'); - } + return md5($url) . '.' . $extension; } else { - $this->logger->error('thumbnail generation error: ' . $url); + $this->logger->warning('Unable to store thumbnail: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/thumbnails.'); } + } else { + $this->logger->error('thumbnail generation error: ' . $url); } return null; @@ -342,34 +348,30 @@ protected function fetchThumbnail($url) { * @return ?string path in the favicons directory */ protected function fetchIcon($url, &$lasticon) { - if (strlen(trim($url)) > 0) { - $format = Image::FORMAT_PNG; - $extension = Image::getExtension($format); - if ($url === $lasticon) { - $this->logger->debug('use last icon: ' . $lasticon); + $format = Image::FORMAT_PNG; + $extension = Image::getExtension($format); + if ($url === $lasticon) { + $this->logger->debug('use last icon: ' . $lasticon); - return md5($lasticon) . '.' . $extension; - } else { - $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); - if ($iconAsPng !== null) { - $written = file_put_contents( - \F3::get('datadir') . '/favicons/' . md5($url) . '.' . $extension, - $iconAsPng - ); - $lasticon = $url; - if ($written !== false) { - $this->logger->debug('Icon generated: ' . $url); - - return md5($url) . '.' . $extension; - } else { - $this->logger->warning('Unable to store icon: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); - } + return md5($lasticon) . '.' . $extension; + } else { + $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); + if ($iconAsPng !== null) { + $written = file_put_contents( + \F3::get('datadir') . '/favicons/' . md5($url) . '.' . $extension, + $iconAsPng + ); + $lasticon = $url; + if ($written !== false) { + $this->logger->debug('Icon generated: ' . $url); + + return md5($url) . '.' . $extension; } else { - $this->logger->error('icon generation error: ' . $url); + $this->logger->warning('Unable to store icon: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); } + } else { + $this->logger->error('icon generation error: ' . $url); } - } else { - $this->logger->debug('no icon for this feed'); } return null; From ef4de7309d889a2143e8dae98c69cddcc8da8701 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Tue, 5 May 2020 00:12:21 +0200 Subject: [PATCH 06/15] helpers\ContentLoader: Simplify icon fetcher even more No weird pass-by-reference any more. --- src/helpers/ContentLoader.php | 48 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index 6601beb1f1..1a12179ec9 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -134,7 +134,7 @@ public function fetch($source) { } $itemsFound = $this->itemsDao->findAll($itemsInFeed, $source['id']); - $lasticon = null; + $iconCache = []; $itemsSeen = []; foreach ($spout as $item) { // item already in database? @@ -211,12 +211,19 @@ public function fetch($source) { try { $iconUrl = $item->getIcon(); if (strlen(trim($iconUrl)) > 0) { - // save icon - $newItem['icon'] = $this->fetchIcon($iconUrl, $lasticon) ?: ''; + if (isset($iconCache[$iconUrl])) { + $this->logger->debug('reusing recently used icon: ' . $iconUrl); + } else { + // save icon + $iconCache[$iconUrl] = $this->fetchIcon($iconUrl) ?: ''; + } + $newItem['icon'] = $iconCache[$iconUrl]; } else { $this->logger->debug('no icon for this feed'); } } catch (\Exception $e) { + // cache failure + $iconCache[$iconUrl] = ''; $this->logger->error('icon: error', ['exception' => $e]); } @@ -343,35 +350,28 @@ protected function fetchThumbnail($url) { * Fetch an image and process it as favicon. * * @param string $url icon given by the spout - * @param &string $lasticon the last fetched icon * * @return ?string path in the favicons directory */ - protected function fetchIcon($url, &$lasticon) { + protected function fetchIcon($url) { $format = Image::FORMAT_PNG; $extension = Image::getExtension($format); - if ($url === $lasticon) { - $this->logger->debug('use last icon: ' . $lasticon); + $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); - return md5($lasticon) . '.' . $extension; - } else { - $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); - if ($iconAsPng !== null) { - $written = file_put_contents( - \F3::get('datadir') . '/favicons/' . md5($url) . '.' . $extension, - $iconAsPng - ); - $lasticon = $url; - if ($written !== false) { - $this->logger->debug('Icon generated: ' . $url); - - return md5($url) . '.' . $extension; - } else { - $this->logger->warning('Unable to store icon: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); - } + if ($iconAsPng !== null) { + $written = file_put_contents( + \F3::get('datadir') . '/favicons/' . md5($url) . '.' . $extension, + $iconAsPng + ); + if ($written !== false) { + $this->logger->debug('Icon generated: ' . $url); + + return md5($url) . '.' . $extension; } else { - $this->logger->error('icon generation error: ' . $url); + $this->logger->warning('Unable to store icon: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); } + } else { + $this->logger->error('icon generation error: ' . $url); } return null; From b13295609141b96bf0d8d12a00f41e76e51dfb54 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Tue, 5 May 2020 01:03:33 +0200 Subject: [PATCH 07/15] spouts: add getSourceIcon() method To allow distinguishing between an icon coming from the feed item and one from the feed itself, we added a new optional getSourceIcon() method. When the getIcon() method returns null for an item or the item icon cannot be fetched, the source icon will be used as a fallback. Both getSourceIcon() and getIcon() can return null now, if an icon is not available. --- NEWS.md | 1 + src/helpers/ContentLoader.php | 21 ++++++++++++++++++++- src/spouts/reddit/reddit2.php | 7 ++----- src/spouts/rss/feed.php | 27 ++++++++++++--------------- src/spouts/spout.php | 17 ++++++++++++++--- 5 files changed, 49 insertions(+), 24 deletions(-) diff --git a/NEWS.md b/NEWS.md index faf9f6eb65..8218da39a0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -57,6 +57,7 @@ ``` ([#1017](https://github.com/SSilence/selfoss/pull/1017), [#1035](https://github.com/SSilence/selfoss/pull/1035)) +- Spouts can now implement `getSourceIcon()` instead of `getIcon()` when icon is associated with the feed, not individual icons. ([#1190](https://github.com/SSilence/selfoss/pull/1190)) - Some language files have been renamed to use correct [IETF language tag](https://en.wikipedia.org/wiki/IETF_language_tag) and you might need to change the `language` key in your `config.ini`: * Simplified Chinese `zh-CN` * Traditional Chinese `zh-TW` diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index 1a12179ec9..e9d19c8d65 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -135,6 +135,7 @@ public function fetch($source) { $itemsFound = $this->itemsDao->findAll($itemsInFeed, $source['id']); $iconCache = []; + $sourceIconUrl = null; $itemsSeen = []; foreach ($spout as $item) { // item already in database? @@ -218,8 +219,26 @@ public function fetch($source) { $iconCache[$iconUrl] = $this->fetchIcon($iconUrl) ?: ''; } $newItem['icon'] = $iconCache[$iconUrl]; + } elseif ($sourceIconUrl !== null) { + $this->logger->debug('using the source icon'); + $newItem['icon'] = $sourceIconUrl; } else { - $this->logger->debug('no icon for this feed'); + try { + // we do not want to run this more than once + $sourceIconUrl = $item->getSourceIcon() ?: ''; + + if (strlen(trim($sourceIconUrl)) > 0) { + // save source icon + $sourceIconUrl = $this->fetchIcon($sourceIconUrl) ?: ''; + $newItem['icon'] = $sourceIconUrl; + } else { + $this->logger->debug('no icon for this item or source'); + } + } catch (\Exception $e) { + // cache failure + $sourceIconUrl = ''; + $this->logger->error('feed icon: error', ['exception' => $e]); + } } } catch (\Exception $e) { // cache failure diff --git a/src/spouts/reddit/reddit2.php b/src/spouts/reddit/reddit2.php index 75287242f6..b4f5692284 100644 --- a/src/spouts/reddit/reddit2.php +++ b/src/spouts/reddit/reddit2.php @@ -54,9 +54,6 @@ class reddit2 extends \spouts\spout { /** @var string the reddit_session cookie */ private $reddit_session = ''; - /** @var string favicon url */ - private $faviconUrl = ''; - /** @var Image image helper */ private $imageHelper; @@ -159,10 +156,10 @@ public function getContent() { public function getIcon() { $htmlUrl = $this->getHtmlUrl(); if ($htmlUrl && ($iconData = $this->imageHelper->fetchFavicon($htmlUrl)) !== null) { - list($this->faviconUrl, $iconBlob) = $iconData; + list($faviconUrl, $iconBlob) = $iconData; } - return $this->faviconUrl; + return $faviconUrl; } public function getLink() { diff --git a/src/spouts/rss/feed.php b/src/spouts/rss/feed.php index 2fe0941d62..df56e5f07a 100644 --- a/src/spouts/rss/feed.php +++ b/src/spouts/rss/feed.php @@ -36,9 +36,6 @@ class feed extends \spouts\spout { /** @var ?string URL of the source */ protected $htmlUrl = null; - /** @var ?string URL of the favicon */ - protected $faviconUrl = null; - /** @var Logger */ private $logger; @@ -103,35 +100,35 @@ public function getContent() { } public function getIcon() { - if ($this->faviconUrl !== null) { - return $this->faviconUrl; - } + return null; + } + public function getSourceIcon() { // Try to use feed logo first $feedLogoUrl = $this->feed->getImageUrl(); if ($feedLogoUrl && ($iconData = $this->imageHelper->fetchFavicon($feedLogoUrl)) !== null) { - list($this->faviconUrl, $iconBlob) = $iconData; - $this->logger->debug('icon: using feed logo: ' . $this->faviconUrl); + list($faviconUrl, $iconBlob) = $iconData; + $this->logger->debug('icon: using feed logo: ' . $faviconUrl); - return $this->faviconUrl; + return $faviconUrl; } // else fallback to the favicon of the associated web page $htmlUrl = $this->getHtmlUrl(); if ($htmlUrl && ($iconData = $this->imageHelper->fetchFavicon($htmlUrl, true)) !== null) { - list($this->faviconUrl, $iconBlob) = $iconData; - $this->logger->debug('icon: using feed homepage favicon: ' . $this->faviconUrl); + list($faviconUrl, $iconBlob) = $iconData; + $this->logger->debug('icon: using feed homepage favicon: ' . $faviconUrl); - return $this->faviconUrl; + return $faviconUrl; } // else fallback to the favicon of the feed effective domain $feedUrl = $this->feed->getFeedUrl(); if ($feedUrl && ($iconData = $this->imageHelper->fetchFavicon($feedUrl, true)) !== null) { - list($this->faviconUrl, $iconBlob) = $iconData; - $this->logger->debug('icon: using feed homepage favicon: ' . $this->faviconUrl); + list($faviconUrl, $iconBlob) = $iconData; + $this->logger->debug('icon: using feed homepage favicon: ' . $faviconUrl); - return $this->faviconUrl; + return $faviconUrl; } return null; diff --git a/src/spouts/spout.php b/src/spouts/spout.php index 75758bd12e..7726eae82c 100644 --- a/src/spouts/spout.php +++ b/src/spouts/spout.php @@ -85,6 +85,15 @@ public function getSpoutTitle() { return $this->spoutTitle; } + /** + * Returns the icon common to this source. + * + * @return ?string icon as URL + */ + public function getSourceIcon() { + return null; + } + /** * returns an unique id for this item * @@ -122,11 +131,13 @@ public function getThumbnail() { } /** - * returns the icon of this item + * Returns the icon of this item. * - * @return string icon as url + * @return ?string icon as url */ - abstract public function getIcon(); + public function getIcon() { + return null; + } /** * returns the link of this item From 39869d4720bc49e682aa03f08c2c7b150314f23e Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 6 May 2020 05:27:20 +0200 Subject: [PATCH 08/15] helpers\Image: Resize SVG like other image formats Only when both width and height are supplied and best fit. --- src/helpers/Image.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/helpers/Image.php b/src/helpers/Image.php index a07d6be8a1..cdc838fbfc 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -181,11 +181,10 @@ public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $heig if ($type === 'svg') { $image = new \Imagick(); $image->readImageBlob($data); - if ($width === null || $height === null) { - $width = $height = 256; + if ($width !== null && $height !== null) { + $image->resizeImage($width, $height, \Imagick::FILTER_LANCZOS, 1, true); } - $image->resizeImage($width, $height, \Imagick::FILTER_LANCZOS, 1); if ($format === self::FORMAT_JPEG) { $image->setImageFormat('jpeg'); $image->setImageCompression(\Imagick::COMPRESSION_JPEG); From 79055433247fde19ff8b5842ea3b735697a4da16 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 6 May 2020 07:16:13 +0200 Subject: [PATCH 09/15] helpers\Image: Fail sooner when dimensionless If the image has no dimensions, it can hardly be resized. --- src/helpers/Image.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/helpers/Image.php b/src/helpers/Image.php index cdc838fbfc..c6cab8f4e1 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -167,7 +167,11 @@ public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $heig } if ($imgInfo === null) { - $imgInfo = @getimagesizefromstring($data); + $imgInfo = getimagesizefromstring($data); + if ($imgInfo[0] === 0 || $imgInfo[1] === 0) { + // unable to determine dimensions + return null; + } } $mimeType = isset($imgInfo['mime']) ? strtolower($imgInfo['mime']) : null; From 1c188574bc078bece7ff4b07f65637b38da61d82 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 6 May 2020 20:42:18 +0200 Subject: [PATCH 10/15] helpers\ContentLoader: factor out favicon/thumbnail storage --- src/common.php | 26 ++++++++++ src/helpers/ContentLoader.php | 79 ++++++++--------------------- src/helpers/IconStore.php | 48 ++++++++++++++++++ src/helpers/Storage/FileStorage.php | 65 ++++++++++++++++++++++++ src/helpers/ThumbnailStore.php | 48 ++++++++++++++++++ 5 files changed, 208 insertions(+), 58 deletions(-) create mode 100644 src/helpers/IconStore.php create mode 100644 src/helpers/Storage/FileStorage.php create mode 100644 src/helpers/ThumbnailStore.php diff --git a/src/common.php b/src/common.php index a2f78c7766..7be0ccc1fe 100644 --- a/src/common.php +++ b/src/common.php @@ -157,6 +157,32 @@ 'constructParams' => $dbParams ])); +$dice->addRule('$iconStorageBackend', [ + 'instanceOf' => helpers\Storage\FileStorage::class, + 'constructParams' => [ + \F3::get('datadir') . '/favicons' + ], +]); + +$dice->addRule(helpers\IconStore::class, array_merge($shared, [ + 'constructParams' => [ + ['instance' => '$iconStorageBackend'], + ] +])); + +$dice->addRule('$thumbnailStorageBackend', [ + 'instanceOf' => helpers\Storage\FileStorage::class, + 'constructParams' => [ + \F3::get('datadir') . '/thumbnails' + ], +]); + +$dice->addRule(helpers\ThumbnailStore::class, array_merge($shared, [ + 'constructParams' => [ + ['instance' => '$thumbnailStorageBackend'], + ] +])); + // Fallback rule $dice->addRule('*', $substitutions); diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index e9d19c8d65..317fb71c8f 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -15,6 +15,9 @@ class ContentLoader { /** @var \daos\Database database for optimization */ private $database; + /** @var IconStore icon store */ + private $iconStore; + /** @var Image image helper */ private $imageHelper; @@ -30,16 +33,24 @@ class ContentLoader { /** @var SpoutLoader spout loader */ private $spoutLoader; + /** @var ThumbnailStore thumbnail store */ + private $thumbnailStore; + + const ICON_FORMAT = Image::FORMAT_PNG; + const THUMBNAIL_FORMAT = Image::FORMAT_JPEG; + /** * ctor */ - public function __construct(\daos\Database $database, Image $imageHelper, \daos\Items $itemsDao, Logger $logger, \daos\Sources $sourcesDao, SpoutLoader $spoutLoader) { + public function __construct(\daos\Database $database, IconStore $iconStore, Image $imageHelper, \daos\Items $itemsDao, Logger $logger, \daos\Sources $sourcesDao, SpoutLoader $spoutLoader, ThumbnailStore $thumbnailStore) { $this->database = $database; + $this->iconStore = $iconStore; $this->imageHelper = $imageHelper; $this->itemsDao = $itemsDao; $this->logger = $logger; $this->sourcesDao = $sourcesDao; $this->spoutLoader = $spoutLoader; + $this->thumbnailStore = $thumbnailStore; } /** @@ -343,21 +354,10 @@ protected function sanitizeField($value) { * @return ?string path in the thumbnails directory */ protected function fetchThumbnail($url) { - $format = Image::FORMAT_JPEG; - $extension = Image::getExtension($format); + $format = self::THUMBNAIL_FORMAT; $thumbnailAsJpg = $this->imageHelper->loadImage($url, $format, 500, 500); if ($thumbnailAsJpg !== null) { - $written = file_put_contents( - \F3::get('datadir') . '/thumbnails/' . md5($url) . '.' . $extension, - $thumbnailAsJpg - ); - if ($written !== false) { - $this->logger->debug('Thumbnail generated: ' . $url); - - return md5($url) . '.' . $extension; - } else { - $this->logger->warning('Unable to store thumbnail: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/thumbnails.'); - } + return $this->thumbnailStore->store($url, $thumbnailAsJpg); } else { $this->logger->error('thumbnail generation error: ' . $url); } @@ -374,21 +374,10 @@ protected function fetchThumbnail($url) { */ protected function fetchIcon($url) { $format = Image::FORMAT_PNG; - $extension = Image::getExtension($format); $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); if ($iconAsPng !== null) { - $written = file_put_contents( - \F3::get('datadir') . '/favicons/' . md5($url) . '.' . $extension, - $iconAsPng - ); - if ($written !== false) { - $this->logger->debug('Icon generated: ' . $url); - - return md5($url) . '.' . $extension; - } else { - $this->logger->warning('Unable to store icon: ' . $url . '. Please check permissions of ' . \F3::get('datadir') . '/favicons.'); - } + return $this->iconStore->store($url, $iconAsPng); } else { $this->logger->error('icon generation error: ' . $url); } @@ -444,12 +433,16 @@ public function cleanup() { // delete orphaned thumbnails $this->logger->debug('delete orphaned thumbnails'); - $this->cleanupFiles('thumbnails'); + $this->thumbnailStore->cleanup(function($file) { + return $this->itemsDao->hasThumbnail($file); + }); $this->logger->debug('delete orphaned thumbnails finished'); // delete orphaned icons $this->logger->debug('delete orphaned icons'); - $this->cleanupFiles('icons'); + $this->iconStore->cleanup(function($file) { + return $this->itemsDao->hasIcon($file); + }); $this->logger->debug('delete orphaned icons finished'); // optimize database @@ -458,36 +451,6 @@ public function cleanup() { $this->logger->debug('optimize database finished'); } - /** - * clean up orphaned thumbnails or icons - * - * @param string $type thumbnails or icons - * - * @return void - */ - protected function cleanupFiles($type) { - if ($type === 'thumbnails') { - $checker = function($file) { - return $this->itemsDao->hasThumbnail($file); - }; - $itemPath = \F3::get('datadir') . '/thumbnails/'; - } elseif ($type === 'icons') { - $checker = function($file) { - return $this->itemsDao->hasIcon($file); - }; - $itemPath = \F3::get('datadir') . '/favicons/'; - } - - foreach (scandir($itemPath) as $file) { - if (is_file($itemPath . $file) && $file !== '.htaccess') { - $inUsage = $checker($file); - if ($inUsage === false) { - unlink($itemPath . $file); - } - } - } - } - /** * Update source (remove previous errors, update last update) * diff --git a/src/helpers/IconStore.php b/src/helpers/IconStore.php new file mode 100644 index 0000000000..83a25548d3 --- /dev/null +++ b/src/helpers/IconStore.php @@ -0,0 +1,48 @@ +storage = $storage; + $this->logger = $logger; + } + + /** + * Store given blob as URL. + * + * @param string $url + * @param string $blob + * + * @return ?string + */ + public function store($url, $blob) { + $extension = Image::getExtension(ContentLoader::ICON_FORMAT); + $this->logger->debug('Storing icon: ' . $url); + + return $this->storage->store($url, $extension, $blob); + } + + /** + * Delete all icons except for requested ones. + * + * @param callable(string):bool $shouldKeep + * + * @return void + */ + public function cleanup($shouldKeep) { + $this->storage->cleanup($shouldKeep); + } +} diff --git a/src/helpers/Storage/FileStorage.php b/src/helpers/Storage/FileStorage.php new file mode 100644 index 0000000000..e873ae1e4a --- /dev/null +++ b/src/helpers/Storage/FileStorage.php @@ -0,0 +1,65 @@ +logger = $logger; + $this->directory = $directory; + } + + /** + * Store given blob with type $extension as URL. + * + * @param string $url + * @param string $extension + * @param string $blob + * + * @return ?string + */ + public function store($url, $extension, $blob) { + $filename = md5($url) . '.' . $extension; + $path = $this->directory . '/' . $filename; + $written = file_put_contents($path, $blob); + + if ($written !== false) { + return $filename; + } else { + $this->logger->warning('Unable to store file: ' . $url . '. Please check permissions of ' . $this->directory); + + return null; + } + } + + /** + * Delete all files except for requested ones. + * + * @param callable(string):bool $shouldKeep + * + * @return ?string + */ + public function cleanup($shouldKeep) { + $itemPath = $this->directory . '/'; + foreach (scandir($itemPath) as $file) { + if (is_file($itemPath . $file) && $file !== '.htaccess') { + if (!$shouldKeep($file)) { + unlink($itemPath . $file); + } + } + } + } +} diff --git a/src/helpers/ThumbnailStore.php b/src/helpers/ThumbnailStore.php new file mode 100644 index 0000000000..057f8a2e92 --- /dev/null +++ b/src/helpers/ThumbnailStore.php @@ -0,0 +1,48 @@ +storage = $storage; + $this->logger = $logger; + } + + /** + * Store given blob as URL. + * + * @param string $url + * @param string $blob + * + * @return ?string + */ + public function store($url, $blob) { + $extension = Image::getExtension(ContentLoader::THUMBNAIL_FORMAT); + $this->logger->debug('Storing thumbnail: ' . $url); + + return $this->storage->store($url, $extension, $blob); + } + + /** + * Delete all icons except for requested ones. + * + * @param callable(string):bool $shouldKeep + * + * @return void + */ + public function cleanup($shouldKeep) { + $this->storage->cleanup($shouldKeep); + } +} From c11e371d0e4ee1dadb61d0fe1b391b302bad2625 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 6 May 2020 21:09:25 +0200 Subject: [PATCH 11/15] helpers: split Image and ImageUtils --- src/helpers/Image.php | 120 +----------------------- src/helpers/ImageUtils.php | 137 ++++++++++++++++++++++++++++ src/spouts/rss/images.php | 2 +- src/spouts/youtube/youtube.php | 2 +- tests/Helpers/IconExtractorTest.php | 28 +++--- 5 files changed, 154 insertions(+), 135 deletions(-) create mode 100644 src/helpers/ImageUtils.php diff --git a/src/helpers/Image.php b/src/helpers/Image.php index c6cab8f4e1..bafff32664 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -109,7 +109,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = } if ($html !== null) { - $shortcutIcons = self::parseShortcutIcons($html); + $shortcutIcons = ImageUtils::parseShortcutIcons($html); foreach ($shortcutIcons as $shortcutIcon) { $shortcutIconUrl = (string) UriResolver::resolve($effectiveUrl, new Uri($shortcutIcon)); @@ -258,122 +258,4 @@ public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $heig return $data; } - - /** - * removes $startString, $endString and everything in between from $subject - * - * @param string $startString - * @param string $endString - * @param string $subject - * - * @return string - */ - public static function cleanString($startString, $endString, $subject) { - while (false !== $p1 = stripos($subject, $startString)) { - $block = substr($subject, $p1); - $subject = substr($subject, 0, $p1); - if (false !== $p2 = stripos($block, $endString)) { - $subject .= substr($block, $p2 + strlen($endString)); - } - } - - return $subject; - } - - /** - * parse shortcut icons from given html - * If icons are found, their URLs are returned in an array ordered by size - * if given or by likely size of not, with the biggest one first. - * - * @param string $html - * - * @return array - */ - public static function parseShortcutIcons($html) { - $end = strripos($html, ''); - if ($end > 1) { - $html = substr($html, 0, $end); - } - - // $html= preg_replace('//sU', '', preg_replace('#<(script|style)\b[^>]*>.*#sU', '', $html)); - // should to the same, but doesn't always. - $html = self::cleanString('', $html); - $html = self::cleanString('', $html); - $html = self::cleanString('', $html); - - // This is only very rough approximation of HTML tag as described by - // https://www.w3.org/TR/2012/WD-html-markup-20120329/syntax.html#syntax-start-tags - // Ideally, we would use a HTML parser but even the streaming parsers I tried - // were several orders of magnitude slower than this mess. - if (preg_match_all('#]*\srel=("|\'|)(?Papple-touch-icon|apple-touch-icon-precomposed|shortcut icon|icon)\1[^>]*>#iU', $html, $links, PREG_SET_ORDER) < 1) { - return []; - } - - $icons = []; - $i = 0; - foreach ($links as $link) { - if (preg_match('#\shref=(?:("|\')(?P.+)\1|(?P[^\s"\'=><`]+?))#iU', $link[0], $href)) { - $icons[] = [ - // Unmatched group should return an empty string but they are missing. - // https://bugs.php.net/bug.php?id=50887 - 'url' => isset($href['uq_url']) && $href['uq_url'] !== '' ? $href['uq_url'] : $href['url'], - // Sizes are only used by Apple. - // https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/ConfiguringWebApplications/ConfiguringWebApplications.html - 'sizes' => preg_match('#\ssizes=("|\'|)(?P[0-9\.]+)x\2\1#i', $link[0], $sizes) - ? $sizes['sizes'] - : 0, - 'rel' => $link['rel'], - 'order' => $i++, - ]; - } - } - if (count($icons) === 0) { - return $icons; - } - - usort($icons, Misc::compareBy( - // largest icons first - [function($val) { - return (int) $val['sizes']; - }, Misc::ORDER_DESC], - - // then by rel priority - [function($val) { - return self::$iconRelWeights[$val['rel']]; - }, Misc::ORDER_DESC], - - // and finally by order to make the sorting stable - 'order' - )); - - return array_map(function($i) { - return $i['url']; - }, $icons); - } - - /** - * taken from: http://zytzagoo.net/blog/2008/01/23/extracting-images-from-html-using-regular-expressions/ - * Searches for the first occurence of an html element in a string - * and extracts the src if it finds it. Returns null in case an - * element is not found. - * - * @param string $html An HTML string - * - * @return ?string content of the src attribute of the first image - */ - public static function findFirstImageSource($html) { - if (stripos($html, ' + */ +class ImageUtils { + private static $iconRelWeights = [ + 'apple-touch-icon-precomposed' => 3, + 'apple-touch-icon' => 2, + 'shortcut icon' => 1, + 'icon' => 1, + ]; + + /** + * removes $startString, $endString and everything in between from $subject + * + * @param string $startString + * @param string $endString + * @param string $subject + * + * @return string + */ + public static function cleanString($startString, $endString, $subject) { + while (false !== $p1 = stripos($subject, $startString)) { + $block = substr($subject, $p1); + $subject = substr($subject, 0, $p1); + if (false !== $p2 = stripos($block, $endString)) { + $subject .= substr($block, $p2 + strlen($endString)); + } + } + + return $subject; + } + + /** + * parse shortcut icons from given html + * If icons are found, their URLs are returned in an array ordered by size + * if given or by likely size of not, with the biggest one first. + * + * @param string $html + * + * @return array + */ + public static function parseShortcutIcons($html) { + $end = strripos($html, ''); + if ($end > 1) { + $html = substr($html, 0, $end); + } + + // $html= preg_replace('//sU', '', preg_replace('#<(script|style)\b[^>]*>.*#sU', '', $html)); + // should to the same, but doesn't always. + $html = self::cleanString('', $html); + $html = self::cleanString('', $html); + $html = self::cleanString('', $html); + + // This is only very rough approximation of HTML tag as described by + // https://www.w3.org/TR/2012/WD-html-markup-20120329/syntax.html#syntax-start-tags + // Ideally, we would use a HTML parser but even the streaming parsers I tried + // were several orders of magnitude slower than this mess. + if (preg_match_all('#]*\srel=("|\'|)(?Papple-touch-icon|apple-touch-icon-precomposed|shortcut icon|icon)\1[^>]*>#iU', $html, $links, PREG_SET_ORDER) < 1) { + return []; + } + + $icons = []; + $i = 0; + foreach ($links as $link) { + if (preg_match('#\shref=(?:("|\')(?P.+)\1|(?P[^\s"\'=><`]+?))#iU', $link[0], $href)) { + $icons[] = [ + // Unmatched group should return an empty string but they are missing. + // https://bugs.php.net/bug.php?id=50887 + 'url' => isset($href['uq_url']) && $href['uq_url'] !== '' ? $href['uq_url'] : $href['url'], + // Sizes are only used by Apple. + // https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/ConfiguringWebApplications/ConfiguringWebApplications.html + 'sizes' => preg_match('#\ssizes=("|\'|)(?P[0-9\.]+)x\2\1#i', $link[0], $sizes) + ? $sizes['sizes'] + : 0, + 'rel' => $link['rel'], + 'order' => $i++, + ]; + } + } + if (count($icons) === 0) { + return $icons; + } + + usort($icons, Misc::compareBy( + // largest icons first + [function($val) { + return (int) $val['sizes']; + }, Misc::ORDER_DESC], + + // then by rel priority + [function($val) { + return self::$iconRelWeights[$val['rel']]; + }, Misc::ORDER_DESC], + + // and finally by order to make the sorting stable + 'order' + )); + + return array_map(function($i) { + return $i['url']; + }, $icons); + } + + /** + * taken from: http://zytzagoo.net/blog/2008/01/23/extracting-images-from-html-using-regular-expressions/ + * Searches for the first occurence of an html element in a string + * and extracts the src if it finds it. Returns null in case an + * element is not found. + * + * @param string $html An HTML string + * + * @return ?string content of the src attribute of the first image + */ + public static function findFirstImageSource($html) { + if (stripos($html, 'get_enclosure(0)->get_link(); } } else { // no enclosures: search image link in content - $image = \helpers\Image::findFirstImageSource(@$item->get_content()); + $image = \helpers\ImageUtils::findFirstImageSource(@$item->get_content()); if ($image !== null) { return $image; } diff --git a/src/spouts/youtube/youtube.php b/src/spouts/youtube/youtube.php index 116337122e..de396fec09 100644 --- a/src/spouts/youtube/youtube.php +++ b/src/spouts/youtube/youtube.php @@ -72,7 +72,7 @@ public function getThumbnail() { return @$item->get_enclosure(0)->get_link(); } } else { // no enclosures: search image link in content - $image = \helpers\Image::findFirstImageSource(@$item->get_content()); + $image = \helpers\ImageUtils::findFirstImageSource(@$item->get_content()); if ($image !== null) { return $image; } diff --git a/tests/Helpers/IconExtractorTest.php b/tests/Helpers/IconExtractorTest.php index 1397220c8f..709a4c929f 100644 --- a/tests/Helpers/IconExtractorTest.php +++ b/tests/Helpers/IconExtractorTest.php @@ -2,7 +2,7 @@ namespace Tests\Helpers; -use helpers\Image; +use helpers\ImageUtils; use PHPUnit\Framework\TestCase; final class IconExtractorTest extends TestCase { @@ -24,7 +24,7 @@ public function testAppleTouchIcon() { [ 'https://www.example.com/images/apple-touch-114x114.png', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -46,7 +46,7 @@ public function testAppleTouchPrecomposedIcon() { [ 'https://www.example.com/images/apple-touch-precomposed-114x114.png', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -68,7 +68,7 @@ public function testAppleTouchWithoutSizesIcon() { [ 'https://www.example.com/images/apple-touch-114x114.png', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -90,7 +90,7 @@ public function testShortcutIcon() { [ 'https://www.example.com/favicon.ico', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -112,7 +112,7 @@ public function testIcon() { [ 'https://www.example.com/favicon.ico', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -138,7 +138,7 @@ public function testMultipleIcons() { '/favicon.ico', '/favicon.svg', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -162,7 +162,7 @@ public function testAppleAndIcon() { 'https://www.example.com/images/apple-touch-114x114.png', '/favicon.ico', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -186,7 +186,7 @@ public function testAppleAndPrecomposed() { 'https://www.example.com/images/apple-touch-114x114.png', 'https://www.example.com/images/apple-touch-precomposed-87x87.png', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -210,7 +210,7 @@ public function testAppleAndPrecomposedWithoutSizes() { 'https://www.example.com/images/apple-touch-precomposed-114x114.png', 'https://www.example.com/images/apple-touch-114x114.png', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -228,7 +228,7 @@ public function testUnquotedAttributes() { '//www.example.com/favicons/apple-touch-icon-152x152.png', '//www.example.com/favicons/favicon.ico', ], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -247,7 +247,7 @@ public function testMissingIcon() { $this->assertEquals( [], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -267,7 +267,7 @@ public function testCommentIcon() { $this->assertEquals( [], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } @@ -289,7 +289,7 @@ public function testScriptIcon() { $this->assertEquals( [], - Image::parseShortcutIcons($page) + ImageUtils::parseShortcutIcons($page) ); } } From 6de93fce14c14ccba2081e8a4e33050c78eed122 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 6 May 2020 21:32:42 +0200 Subject: [PATCH 12/15] helpers\Image: loadImage should not fetch URLs We need the blob for content type detection so rather than downloading it twice, let's pass it to loadImage directly. --- src/helpers/ContentLoader.php | 45 +++++++++++++++------- src/helpers/Image.php | 72 ++++++++++++++++++----------------- 2 files changed, 70 insertions(+), 47 deletions(-) diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index 317fb71c8f..cdc32a8a67 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -36,13 +36,16 @@ class ContentLoader { /** @var ThumbnailStore thumbnail store */ private $thumbnailStore; + /** @var WebClient thumbnail store */ + private $webClient; + const ICON_FORMAT = Image::FORMAT_PNG; const THUMBNAIL_FORMAT = Image::FORMAT_JPEG; /** * ctor */ - public function __construct(\daos\Database $database, IconStore $iconStore, Image $imageHelper, \daos\Items $itemsDao, Logger $logger, \daos\Sources $sourcesDao, SpoutLoader $spoutLoader, ThumbnailStore $thumbnailStore) { + public function __construct(\daos\Database $database, IconStore $iconStore, Image $imageHelper, \daos\Items $itemsDao, Logger $logger, \daos\Sources $sourcesDao, SpoutLoader $spoutLoader, ThumbnailStore $thumbnailStore, WebClient $webClient) { $this->database = $database; $this->iconStore = $iconStore; $this->imageHelper = $imageHelper; @@ -51,6 +54,7 @@ public function __construct(\daos\Database $database, IconStore $iconStore, Imag $this->sourcesDao = $sourcesDao; $this->spoutLoader = $spoutLoader; $this->thumbnailStore = $thumbnailStore; + $this->webClient = $webClient; } /** @@ -354,12 +358,20 @@ protected function sanitizeField($value) { * @return ?string path in the thumbnails directory */ protected function fetchThumbnail($url) { - $format = self::THUMBNAIL_FORMAT; - $thumbnailAsJpg = $this->imageHelper->loadImage($url, $format, 500, 500); - if ($thumbnailAsJpg !== null) { - return $this->thumbnailStore->store($url, $thumbnailAsJpg); - } else { - $this->logger->error('thumbnail generation error: ' . $url); + try { + $data = $this->webClient->request($url); + $format = self::THUMBNAIL_FORMAT; + $thumbnailAsJpg = $this->imageHelper->loadImage($data, $format, 500, 500); + + if ($thumbnailAsJpg !== null) { + return $this->thumbnailStore->store($url, $thumbnailAsJpg); + } else { + $this->logger->error('thumbnail generation error: ' . $url); + } + } catch (\Exception $e) { + $this->logger->error("failed to retrieve thumbnail $url,", ['exception' => $e]); + + return null; } return null; @@ -373,13 +385,20 @@ protected function fetchThumbnail($url) { * @return ?string path in the favicons directory */ protected function fetchIcon($url) { - $format = Image::FORMAT_PNG; - $iconAsPng = $this->imageHelper->loadImage($url, $format, 30, null); + try { + $data = $this->webClient->request($url); + $format = Image::FORMAT_PNG; + $iconAsPng = $this->imageHelper->loadImage($data, $format, 30, null); + + if ($iconAsPng !== null) { + return $this->iconStore->store($url, $iconAsPng); + } else { + $this->logger->error('icon generation error: ' . $url); + } + } catch (\Exception $e) { + $this->logger->error("failed to retrieve image $url,", ['exception' => $e]); - if ($iconAsPng !== null) { - return $this->iconStore->store($url, $iconAsPng); - } else { - $this->logger->error('icon generation error: ' . $url); + return null; } return null; diff --git a/src/helpers/Image.php b/src/helpers/Image.php index bafff32664..ee16377489 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -84,48 +84,61 @@ public static function getExtension($format) { */ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = null) { // try given url - if ($isHtmlUrl === false) { - $faviconAsPng = $this->loadImage($url, self::FORMAT_PNG, $width, $height); - if ($faviconAsPng !== null) { - return [$url, $faviconAsPng]; - } - } - - $urlElements = parse_url($url); - - // search on base page for webClient->getHttpClient(); $response = $http->get($url); - $html = (string) $response->getBody(); + $blob = (string) $response->getBody(); $effectiveUrl = new Uri(WebClient::getEffectiveUrl($url, $response)); if ($response->getStatusCode() !== 200) { - throw new \Exception(substr($html, 0, 512)); + throw new \Exception(substr($blob, 0, 512)); } } catch (\Exception $e) { - $this->logger->debug('icon: failed to get html page: ', ['exception' => $e]); + $this->logger->error("icon: failed to retrieve URL $url", ['exception' => $e]); + + return null; + } + + if ($isHtmlUrl === false) { + $faviconAsPng = $this->loadImage($blob, self::FORMAT_PNG, $width, $height); + if ($faviconAsPng !== null) { + return [$url, $faviconAsPng]; + } } - if ($html !== null) { - $shortcutIcons = ImageUtils::parseShortcutIcons($html); + // When HTML page, search for icon links + if (preg_match('#^text/html\b#i', $response->getHeaderLine('content-type')) || preg_match('#]#si', $blob)) { + $shortcutIcons = ImageUtils::parseShortcutIcons($blob); foreach ($shortcutIcons as $shortcutIcon) { $shortcutIconUrl = (string) UriResolver::resolve($effectiveUrl, new Uri($shortcutIcon)); - $faviconAsPng = $this->loadImage($shortcutIconUrl, self::FORMAT_PNG, $width, $height); - if ($faviconAsPng !== null) { - return [$shortcutIconUrl, $faviconAsPng]; + try { + $data = $this->webClient->request($shortcutIconUrl); + $faviconAsPng = $this->loadImage($data, self::FORMAT_PNG, $width, $height); + + if ($faviconAsPng !== null) { + return [$shortcutIconUrl, $faviconAsPng]; + } + } catch (\Exception $e) { + $this->logger->error("failed to retrieve image $url,", ['exception' => $e]); } } } + $urlElements = parse_url($url); + // search domain/favicon.ico if (isset($urlElements['scheme']) && isset($urlElements['host'])) { $url = $urlElements['scheme'] . '://' . $urlElements['host'] . '/favicon.ico'; - $faviconAsPng = $this->loadImage($url, self::FORMAT_PNG, $width, $height); - if ($faviconAsPng !== null) { - return [$url, $faviconAsPng]; + try { + $data = $this->webClient->request($url); + $faviconAsPng = $this->loadImage($data, self::FORMAT_PNG, $width, $height); + + if ($faviconAsPng !== null) { + return [$url, $faviconAsPng]; + } + } catch (\Exception $e) { + $this->logger->error("failed to retrieve image $url,", ['exception' => $e]); } } @@ -135,23 +148,14 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = /** * Load image from URL, optionally resize it and convert it to desired format. * - * @param string $url source url + * @param string $data * @param self::FORMAT_JPEG|self::FORMAT_PNG $format file format of output file * @param ?int $width target width * @param ?int $height target height * * @return ?string blob containing the processed image */ - public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $height = null) { - // load image - try { - $data = $this->webClient->request($url); - } catch (\Exception $e) { - $this->logger->error("failed to retrieve image $url,", ['exception' => $e]); - - return null; - } - + public function loadImage($data, $format = self::FORMAT_PNG, $width = null, $height = null) { $imgInfo = null; // get image type @@ -208,7 +212,7 @@ public function loadImage($url, $format = self::FORMAT_PNG, $width = null, $heig try { $icon = $loader->fromString($data); } catch (\InvalidArgumentException $e) { - $this->logger->error("Icon “{$url}” is not valid", ['exception' => $e]); + $this->logger->error('Icon is not valid', ['exception' => $e]); return null; } From 974937dede7c42f98999dc79db9aea513d220a7d Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 13 Sep 2020 00:08:30 +0200 Subject: [PATCH 13/15] helpers\Image: Fix false positives for SVG When SVG element was embedded in a HTML document, it was detected as SVG image instead of a HTML page, causing issues for favicon fetching. Fixes: https://github.com/SSilence/selfoss/issues/1191 --- src/helpers/Image.php | 2 +- src/helpers/ImageUtils.php | 11 +++++ tests/Helpers/DetectSvgTest.php | 78 +++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 tests/Helpers/DetectSvgTest.php diff --git a/src/helpers/Image.php b/src/helpers/Image.php index ee16377489..faf46ccb48 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -165,7 +165,7 @@ public function loadImage($data, $format = self::FORMAT_PNG, $width = null, $hei $data = $d; } - if (preg_match('#]#si', $data)) { + if (ImageUtils::detectSvg($data)) { $imgInfo = ['mime' => 'image/svg+xml']; } } diff --git a/src/helpers/ImageUtils.php b/src/helpers/ImageUtils.php index fa762b8ca6..416f283000 100644 --- a/src/helpers/ImageUtils.php +++ b/src/helpers/ImageUtils.php @@ -134,4 +134,15 @@ public static function findFirstImageSource($html) { return null; } } + + /** + * Detect if given data is an SVG file and not just a HTML document with SVG embedded. + * + * @param string $blob + * + * @return bool true when it is a SVG + */ + public static function detectSvg($blob) { + return preg_match('#]#si', $blob) && !preg_match('#<((!doctype )?html|body)[\s>]#si', $blob); + } } diff --git a/tests/Helpers/DetectSvgTest.php b/tests/Helpers/DetectSvgTest.php new file mode 100644 index 0000000000..a0bbe97a4f --- /dev/null +++ b/tests/Helpers/DetectSvgTest.php @@ -0,0 +1,78 @@ + +EOD; + + $this->assertTrue( + ImageUtils::detectSvg($blob) + ); + } + + /** + * Detects a SVG file with XML directives correctly. + */ + public function testDirectives() { + $blob = << + + + +EOD; + + $this->assertTrue( + ImageUtils::detectSvg($blob) + ); + } + + /** + * Detects a SVG embedded in HTML file correctly. + */ + public function testInHtml() { + $blob = << + +EOD; + + $this->assertFalse( + ImageUtils::detectSvg($blob) + ); + } + + /** + * Detects a SVG embedded in HTML file with a doctype correctly. + */ + public function testInHtmlWithDoctype() { + $blob = <<Foo +EOD; + + $this->assertFalse( + ImageUtils::detectSvg($blob) + ); + } + + /** + * Detects a SVG embedded in HTML file with just a body correctly. + */ + public function testInHtmlWithBody() { + $blob = << +EOD; + + $this->assertFalse( + ImageUtils::detectSvg($blob) + ); + } +} From 99d42da265d4d3fe57ffd3f9c01ae0afaa6d455f Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 13 Sep 2020 00:53:36 +0200 Subject: [PATCH 14/15] spouts\rss: Prefer site icon over rectangular logo Some sites like golem.de use rectangular logos in feeds, which are unsuitable for feed icon. Let's fall back to favicon instead. Fixes: https://github.com/SSilence/selfoss/issues/1183 --- src/helpers/ContentLoader.php | 12 +++++------ src/helpers/Image.php | 26 +++++++++++------------ src/helpers/ImageHolder.php | 40 +++++++++++++++++++++++++++++++++++ src/spouts/rss/feed.php | 11 ++++++++-- 4 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 src/helpers/ImageHolder.php diff --git a/src/helpers/ContentLoader.php b/src/helpers/ContentLoader.php index cdc32a8a67..d50d5e14cb 100644 --- a/src/helpers/ContentLoader.php +++ b/src/helpers/ContentLoader.php @@ -361,10 +361,10 @@ protected function fetchThumbnail($url) { try { $data = $this->webClient->request($url); $format = self::THUMBNAIL_FORMAT; - $thumbnailAsJpg = $this->imageHelper->loadImage($data, $format, 500, 500); + $image = $this->imageHelper->loadImage($data, $format, 500, 500); - if ($thumbnailAsJpg !== null) { - return $this->thumbnailStore->store($url, $thumbnailAsJpg); + if ($image !== null) { + return $this->thumbnailStore->store($url, $image->getData()); } else { $this->logger->error('thumbnail generation error: ' . $url); } @@ -388,10 +388,10 @@ protected function fetchIcon($url) { try { $data = $this->webClient->request($url); $format = Image::FORMAT_PNG; - $iconAsPng = $this->imageHelper->loadImage($data, $format, 30, null); + $image = $this->imageHelper->loadImage($data, $format, 30, null); - if ($iconAsPng !== null) { - return $this->iconStore->store($url, $iconAsPng); + if ($image !== null) { + return $this->iconStore->store($url, $image->getData()); } else { $this->logger->error('icon generation error: ' . $url); } diff --git a/src/helpers/Image.php b/src/helpers/Image.php index faf46ccb48..b98efe863a 100644 --- a/src/helpers/Image.php +++ b/src/helpers/Image.php @@ -80,7 +80,7 @@ public static function getExtension($format) { * @param ?int $width * @param ?int $height * - * @return ?array{string, string} pair of URL and blob containing the image data + * @return ?array{string, ImageHolder} pair of URL and blob containing the image data */ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = null) { // try given url @@ -100,9 +100,9 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = } if ($isHtmlUrl === false) { - $faviconAsPng = $this->loadImage($blob, self::FORMAT_PNG, $width, $height); - if ($faviconAsPng !== null) { - return [$url, $faviconAsPng]; + $image = $this->loadImage($blob, self::FORMAT_PNG, $width, $height); + if ($image !== null) { + return [$url, $image]; } } @@ -114,10 +114,10 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = try { $data = $this->webClient->request($shortcutIconUrl); - $faviconAsPng = $this->loadImage($data, self::FORMAT_PNG, $width, $height); + $image = $this->loadImage($data, self::FORMAT_PNG, $width, $height); - if ($faviconAsPng !== null) { - return [$shortcutIconUrl, $faviconAsPng]; + if ($image !== null) { + return [$shortcutIconUrl, $image]; } } catch (\Exception $e) { $this->logger->error("failed to retrieve image $url,", ['exception' => $e]); @@ -132,10 +132,10 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = $url = $urlElements['scheme'] . '://' . $urlElements['host'] . '/favicon.ico'; try { $data = $this->webClient->request($url); - $faviconAsPng = $this->loadImage($data, self::FORMAT_PNG, $width, $height); + $image = $this->loadImage($data, self::FORMAT_PNG, $width, $height); - if ($faviconAsPng !== null) { - return [$url, $faviconAsPng]; + if ($image !== null) { + return [$url, $image]; } } catch (\Exception $e) { $this->logger->error("failed to retrieve image $url,", ['exception' => $e]); @@ -153,7 +153,7 @@ public function fetchFavicon($url, $isHtmlUrl = false, $width = null, $height = * @param ?int $width target width * @param ?int $height target height * - * @return ?string blob containing the processed image + * @return ?ImageHolder blob containing the processed image */ public function loadImage($data, $format = self::FORMAT_PNG, $width = null, $height = null) { $imgInfo = null; @@ -203,7 +203,7 @@ public function loadImage($data, $format = self::FORMAT_PNG, $width = null, $hei $image->setOption('png:compression-level', 9); } - return $image; + return new ImageHolder((string) $image, $format, $image->getImageWidth(), $image->getImageHeight()); } // convert ico to png @@ -260,6 +260,6 @@ public function loadImage($data, $format = self::FORMAT_PNG, $width = null, $hei $data = $wideImage->asString('png', 4, PNG_NO_FILTER); } - return $data; + return new ImageHolder($data, $format, $wideImage->getWidth(), $wideImage->getHeight()); } } diff --git a/src/helpers/ImageHolder.php b/src/helpers/ImageHolder.php new file mode 100644 index 0000000000..a72ba65f3c --- /dev/null +++ b/src/helpers/ImageHolder.php @@ -0,0 +1,40 @@ +data = $data; + $this->format = $format; + $this->width = $width; + $this->height = $height; + } + + public function getData() { + return $this->data; + } + + public function getFormat() { + return $this->format; + } + + public function getWidth() { + return $this->width; + } + + public function getHeight() { + return $this->height; + } +} diff --git a/src/spouts/rss/feed.php b/src/spouts/rss/feed.php index df56e5f07a..e53f8e3b4a 100644 --- a/src/spouts/rss/feed.php +++ b/src/spouts/rss/feed.php @@ -108,9 +108,16 @@ public function getSourceIcon() { $feedLogoUrl = $this->feed->getImageUrl(); if ($feedLogoUrl && ($iconData = $this->imageHelper->fetchFavicon($feedLogoUrl)) !== null) { list($faviconUrl, $iconBlob) = $iconData; - $this->logger->debug('icon: using feed logo: ' . $faviconUrl); - return $faviconUrl; + $aspectRatio = $iconBlob->getWidth() / $iconBlob->getHeight(); + + if (0.8 < $aspectRatio && $aspectRatio < 1.3) { + $this->logger->debug('icon: using feed logo: ' . $faviconUrl); + + return $faviconUrl; + } else { + $this->logger->debug('icon: feed logo “' . $faviconUrl . '” not square enough with aspect ratio ' . $aspectRatio . '. Not using it.'); + } } // else fallback to the favicon of the associated web page From 2eac971772a2fcee8282f5a0a416d9324c757e33 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sun, 13 Sep 2020 10:49:55 +0200 Subject: [PATCH 15/15] spouts\reddit: Fix infinite loop when no children Reddit for some reason returns a JSON string "{}" when trying to fetch `message/inbox`, which causes the ItemsIterator to freak out and loop without an end. --- src/spouts/reddit/reddit2.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/spouts/reddit/reddit2.php b/src/spouts/reddit/reddit2.php index b4f5692284..c45b7fcd81 100644 --- a/src/spouts/reddit/reddit2.php +++ b/src/spouts/reddit/reddit2.php @@ -89,7 +89,9 @@ public function load(array $params) { throw new \Exception($json['message']); } - $this->items = $json['data']['children']; + if (isset($json['data']) && isset($json['data']['children'])) { + $this->items = $json['data']['children']; + } } public function getId() {