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/';