From 7d61a1cb09f792cbf0fbfc2eb070693c04533501 Mon Sep 17 00:00:00 2001 From: Benjamin Brahmer Date: Fri, 15 Oct 2021 10:52:16 +0200 Subject: Download feed logos via guzzle to have better error handling Signed-off-by: Benjamin Brahmer --- AUTHORS.md | 10 ++++- CHANGELOG.md | 2 + composer.json | 18 ++++---- composer.lock | 3 +- lib/Fetcher/FeedFetcher.php | 76 +++++++++++++++++++++++++++++----- tests/Unit/Fetcher/FeedFetcherTest.php | 12 +++++- 6 files changed, 97 insertions(+), 24 deletions(-) diff --git a/AUTHORS.md b/AUTHORS.md index afeda70f2..fba40eaf0 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -6,9 +6,9 @@ * [Robin Appelman](mailto:icewind@owncloud.com) * [Sean Molenaar](mailto:sean@seanmolenaar.eu) * [Gregor Tätzner](mailto:gregor@freenet.de) +* [anoy](mailto:anoymouserver+github@mailbox.org) * [Sean Molenaar](mailto:SMillerDev@users.noreply.github.com) * [Morris Jobke](mailto:hey@morrisjobke.de) -* [anoy](mailto:anoymouserver+github@mailbox.org) * [Jimmy Huynh](mailto:jimmy.huynh@etu.unistra.fr) * [Aurélien](mailto:dav.aurelien@gmail.com) * [Jan-Christoph Borchardt](mailto:hey@jancborchardt.net) @@ -29,6 +29,7 @@ * [IBBoard](mailto:dev@ibboard.co.uk) * [Koen Martens](mailto:gmc@sonologic.nl) * [Lukas Reschke](mailto:lukas@owncloud.com) +* [Tucker McKnight](mailto:tucker.mcknight@gmail.com) * [Bart Visscher](mailto:bartv@thisnet.nl) * [Christian Elmer](mailto:christian@keinkurt.de) * [Nicolas Wendling](mailto:nicolas.wendling1011@gmail.com) @@ -36,6 +37,7 @@ * [Volkan Gezer](mailto:volkangezer@gmail.com) * [Xéfir Destiny](mailto:xefir@crystalyx.net) * [Daniel Opitz](mailto:danopz@users.noreply.github.com) +* [Daniel Rheinbay](mailto:danielrheinbay@gmail.com) * [Lars Bensmann](mailto:lars@almosthappy.de) * [Robin Appelman](mailto:robin@icewind.nl) * [bluehaze](mailto:francesco.sportolari@gmail.com) @@ -73,6 +75,7 @@ * [davidak](mailto:git@davidak.de) * [lsmooth](mailto:ls@lsmooth.de) * [s17t.net](mailto:mail+github@s17t.net) +* [Alec Kojaev](mailto:alec@kojaev.name) * [Alessandro](mailto:cosenal@gmail.com) * [Alexander Grüßung](mailto:alexander@gruessung-online.de) * [Allan Nordhøy](mailto:epost@anotheragency.no) @@ -95,6 +98,7 @@ * [Colin W](mailto:cwmke@users.noreply.github.com) * [Daniel Aleksandersen](mailto:code@daniel.priv.no) * [Daniel S](mailto:daniel@while-true-do.org) +* [David Baucum](mailto:david@baucum.me) * [David Engster](mailto:deng@randomsample.de) * [Detlev Zundel](mailto:dzu@member.fsf.org) * [Doron Behar](mailto:doron.behar@gmail.com) @@ -128,6 +132,7 @@ * [Pierre Ozoux](mailto:pierre@ozoux.net) * [Piotr Dobrowolski](mailto:admin@tastycode.pl) * [Raspbeguy](mailto:raspbeguy@users.noreply.github.com) +* [René Henrich](mailto:contact@rene-henrich.de) * [Rodrigo Aguilera](mailto:rodrigo.aguilera@amazee.com) * [Roeland Jago Douma](mailto:roeland@famdouma.nl) * [Simon](mailto:sschubert89@gmail.com) @@ -140,13 +145,13 @@ * [Thomas Wouters](mailto:twouters@users.noreply.github.com) * [Tilo Spannagel](mailto:development@tilosp.de) * [Timo Schmidt](mailto:timo@xinterchange.net) -* [Tucker McKnight](mailto:tucker.mcknight@gmail.com) * [WENDLING NICOLAS](mailto:nicolas.wendling1011@gmail.com) * [Welling Guzmán](mailto:WellingGuzman@users.noreply.github.com) * [Xaver Maierhofer](mailto:xaver.maierhofer@xwissen.info) * [Xemle](mailto:xemle@phtagr.org) * [YMHuang](mailto:ymhuang@fmbase.tw) * [Zach DeCook](mailto:zachdecook@gmail.com) +* [Zach DeCook](mailto:zachdecook@librem.one) * [amittel](mailto:arnulf.mittelstaedt@gmail.com) * [b_b](mailto:brunobergot@gmail.com) * [bjoerns1983](mailto:bjoern@sengotta.net) @@ -161,6 +166,7 @@ * [kondou](mailto:kondou@ts.unde.re) * [markusj](mailto:markusj@users.noreply.github.com) * [mnassabain](mailto:34754819+mnassabain@users.noreply.github.com) +* [mormegil](mailto:mormegil@centrum.cz) * [nexus-uw](mailto:you@example.com) * [repat](mailto:repat@repat.de) * [ritchiewilson](mailto:rawilson52@gmail.com) diff --git a/CHANGELOG.md b/CHANGELOG.md index c227ebf8d..2266b1509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,11 @@ The format is almost based on [Keep a Changelog](https://keepachangelog.com/en/1 ## [16.x.x] ### Changed - Add changelog and DCO notice to CONTRIBUTING.md (#1521) +- Download feed logos via guzzle to have better error handling (#1533) ### Fixed + # Releases ## [16.1.0] - 2021-10-07 ### Changed diff --git a/composer.json b/composer.json index 1eb4d0857..aa52f8128 100644 --- a/composer.json +++ b/composer.json @@ -55,15 +55,15 @@ }, "require-dev": { "phpunit/phpunit": "9.5.*", - "squizlabs/php_codesniffer": "^3.5.6", - "phpstan/phpstan": "^0.12.43", - "phpstan/phpstan-doctrine": "^0.12.22", - "phpstan/phpstan-strict-rules": "^0.12.5", - "phpstan/phpstan-phpunit": "^0.12.16", - "phpstan/extension-installer": "^1.0", - "guzzlehttp/guzzle": "^7.2", - "doctrine/dbal": "^3.0", - "symfony/console": "^4.4.18", + "squizlabs/php_codesniffer": "^3.6.1", + "phpstan/phpstan": "^0.12.99", + "phpstan/phpstan-doctrine": "^0.12.44", + "phpstan/phpstan-strict-rules": "^0.12.11", + "phpstan/phpstan-phpunit": "^0.12.22", + "phpstan/extension-installer": "^1.1.0", + "guzzlehttp/guzzle": "^7.3.0", + "doctrine/dbal": "^3.1.3", + "symfony/console": "^4.4.19", "psr/log": "^1.1.0" }, "replace": { diff --git a/composer.lock b/composer.lock index 2f73fe9ec..060ac69d5 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1932196eb576f7eb70b3846487613b5a", + "content-hash": "6487fb576db846a56aaf0d45f2469a8f", "packages": [ { "name": "arthurhoaro/favicon", @@ -2864,6 +2864,7 @@ "type": "github" } ], + "abandoned": true, "time": "2020-09-28T06:45:17+00:00" }, { diff --git a/lib/Fetcher/FeedFetcher.php b/lib/Fetcher/FeedFetcher.php index ef232eece..9cab0287d 100755 --- a/lib/Fetcher/FeedFetcher.php +++ b/lib/Fetcher/FeedFetcher.php @@ -18,6 +18,8 @@ use Favicon\Favicon; use FeedIo\Feed\ItemInterface; use FeedIo\FeedInterface; use FeedIo\FeedIo; +use GuzzleHttp\Client; +use GuzzleHttp\Exception\RequestException; use Net_URL2; use OCP\IL10N; @@ -68,6 +70,11 @@ class FeedFetcher implements IFeedFetcher */ private $logger; + /** + * @var Client + */ + private $client; + public function __construct( FeedIo $fetcher, Favicon $favicon, @@ -75,7 +82,8 @@ class FeedFetcher implements IFeedFetcher IL10N $l10n, ITempManager $ITempManager, Time $time, - LoggerInterface $logger + LoggerInterface $logger, + Client $client ) { $this->reader = $fetcher; $this->faviconFactory = $favicon; @@ -84,6 +92,7 @@ class FeedFetcher implements IFeedFetcher $this->ITempManager = $ITempManager; $this->time = $time; $this->logger = $logger; + $this->client = $client; } @@ -318,14 +327,15 @@ class FeedFetcher implements IFeedFetcher /** * Return the favicon for a given feed and url * - * @param FeedInterface $feed Feed to check for a logo - * @param string $url Original URL for the feed + * @param FeedInterface $feed Feed to check for a logo + * @param string $url Original URL for the feed * * @return string|mixed|bool */ protected function getFavicon(FeedInterface $feed, string $url) { - $favicon = $feed->getLogo(); + // trim the string because authors do funny things + $favicon = trim($feed->getLogo()); ini_set('user_agent', 'NextCloud-News/1.0'); @@ -334,17 +344,61 @@ class FeedFetcher implements IFeedFetcher $base_url = $base_url->getNormalizedURL(); // check if feed has a logo entry - if (is_null($favicon) || trim($favicon) === '') { + if (is_null($favicon) || $favicon === '') { return $this->faviconFactory->get($base_url); } - $favicon_path = join("/", [$this->ITempManager->getTempBaseDir(), basename($favicon)]); + // logo will be saved in the tmp folder provided by Nextcloud, file is named as md5 of the url + $favicon_path = join("/", [$this->ITempManager->getTempBaseDir(), md5($favicon)]); + $downloaded = false; - $downloaded = copy( - $favicon, - $favicon_path, - stream_context_create([ 'http' => [ 'ignore_errors' => true ] ]) - ); + if (file_exists($favicon_path)) { + $last_modified = filemtime($favicon_path); + } else { + $last_modified = 0; + } + + try { + $response = $this->client->request( + 'GET', + $favicon, + [ + 'sink' => $favicon_path, + 'headers' => [ + 'User-Agent' => 'NextCloud-News/1.0', + 'Accept' => 'image/*', + 'If-Modified-Since' => date(DateTime::RFC7231, $last_modified) + ] + ] + ); + $downloaded = true; + + $this->logger->debug( + "Feed:{url} Logo:{logo} Status:{status}", + [ + 'status' => $response->getStatusCode(), + 'url' => $favicon_path, + 'logo' => $favicon + ] + ); + } catch (RequestException $e) { + if ($e->hasResponse()) { + $this->logger->info( + 'An error occurred while trying to download the feed logo of {url}: {error}', + [ + 'url' => $url, + 'error' => $e->getResponse() + ] + ); + } else { + $this->logger->info( + 'An unknown error occurred while trying to download the feed logo of {url}.', + [ + 'url' => $url, + ] + ); + } + } $is_image = $downloaded && substr(mime_content_type($favicon_path), 0, 5) === "image"; diff --git a/tests/Unit/Fetcher/FeedFetcherTest.php b/tests/Unit/Fetcher/FeedFetcherTest.php index 2b77b00e0..32b03d3e4 100644 --- a/tests/Unit/Fetcher/FeedFetcherTest.php +++ b/tests/Unit/Fetcher/FeedFetcherTest.php @@ -28,6 +28,7 @@ use \OCA\News\Db\Feed; use \OCA\News\Db\Item; use OCA\News\Scraper\Scraper; use OCA\News\Fetcher\FeedFetcher; +use GuzzleHttp\Client; use OCA\News\Utility\Time; use OCP\IL10N; @@ -106,6 +107,11 @@ class FeedFetcherTest extends TestCase */ private $scraper; + /** + * @var MockObject|Client + */ + private $client; + //metadata /** * @var integer @@ -188,6 +194,9 @@ class FeedFetcherTest extends TestCase $this->scraper = $this->getMockBuilder(Scraper::class) ->disableOriginalConstructor() ->getMock(); + $this->client = $this->getMockBuilder(Client::class) + ->disableOriginalConstructor() + ->getMock(); $this->fetcher = new FeedFetcher( $this->reader, $this->favicon, @@ -195,7 +204,8 @@ class FeedFetcherTest extends TestCase $this->l10n, $this->ITempManager, $timeFactory, - $this->logger + $this->logger, + $this->client ); $this->url = 'http://tests/'; -- cgit v1.2.3