From e563de4de790d7970bf5f8f52a7fac27f196af4e Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 28 Dec 2022 19:09:34 -0500 Subject: [PATCH] Ward againt contact_id = 0 in UpdateContact worker - Add parameter validation in Worker\UpdateContact::add - Address https://github.com/friendica/friendica/issues/12487#issuecomment-1366833644 --- src/Model/Contact.php | 27 ++++++++++++++++++++++----- src/Module/Contact.php | 9 ++++++++- src/Module/Photo.php | 9 +++++++-- src/Worker/UpdateContact.php | 21 ++++++++++++++++++++- src/Worker/UpdateContacts.php | 22 ++++++++++++++-------- 5 files changed, 71 insertions(+), 17 deletions(-) diff --git a/src/Model/Contact.php b/src/Model/Contact.php index bf90edcd4c..7a0debc98a 100644 --- a/src/Model/Contact.php +++ b/src/Model/Contact.php @@ -47,6 +47,7 @@ use Friendica\Util\Images; use Friendica\Util\Network; use Friendica\Util\Proxy; use Friendica\Util\Strings; +use Friendica\Worker\UpdateContact; /** * functions for interacting with a contact @@ -362,7 +363,11 @@ class Contact // Update the contact in the background if needed if ($background_update && !self::isLocal($url) && Probe::isProbable($contact['network']) && ($contact['next-update'] < DateTimeFormat::utcNow())) { - Worker::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], 'UpdateContact', $contact['id']); + try { + UpdateContact::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], $contact['id']); + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['contact' => $contact]); + } } // Remove the internal fields @@ -1276,7 +1281,11 @@ class Contact $background_update = DI::config()->get('system', 'update_active_contacts') ? $contact['local-data'] : true; if ($background_update && !self::isLocal($url) && Probe::isProbable($contact['network']) && ($contact['next-update'] < DateTimeFormat::utcNow())) { - Worker::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], 'UpdateContact', $contact['id']); + try { + UpdateContact::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], $contact['id']); + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['contact' => $contact]); + } } if (empty($update) && (!empty($contact['uri-id']) || is_bool($update))) { @@ -3078,7 +3087,11 @@ class Contact if ($probed) { self::updateFromProbeArray($contact_id, $ret); } else { - Worker::add(Worker::PRIORITY_HIGH, 'UpdateContact', $contact_id); + try { + UpdateContact::add(Worker::PRIORITY_HIGH, $contact['id']); + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['contact' => $contact]); + } } $result['success'] = Protocol::follow($uid, $contact, $protocol); @@ -3549,8 +3562,12 @@ class Contact Worker::add(Worker::PRIORITY_LOW, 'AddContact', 0, $url); ++$added; } elseif (!empty($contact['network']) && Probe::isProbable($contact['network']) && ($contact['next-update'] < DateTimeFormat::utcNow())) { - Worker::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], 'UpdateContact', $contact['id']); - ++$updated; + try { + UpdateContact::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], $contact['id']); + ++$updated; + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['contact' => $contact]); + } } else { ++$unchanged; } diff --git a/src/Module/Contact.php b/src/Module/Contact.php index e8c9d2241a..4b2d8edad7 100644 --- a/src/Module/Contact.php +++ b/src/Module/Contact.php @@ -26,8 +26,10 @@ use Friendica\Content\ContactSelector; use Friendica\Content\Nav; use Friendica\Content\Pager; use Friendica\Content\Widget; +use Friendica\Core\Logger; use Friendica\Core\Protocol; use Friendica\Core\Renderer; +use Friendica\Core\System; use Friendica\Core\Theme; use Friendica\Core\Worker; use Friendica\Database\DBA; @@ -37,6 +39,7 @@ use Friendica\Model\User; use Friendica\Module\Security\Login; use Friendica\Network\HTTPException\InternalServerErrorException; use Friendica\Network\HTTPException\NotFoundException; +use Friendica\Worker\UpdateContact; /** * Manages and show Contacts and their content @@ -129,7 +132,11 @@ class Contact extends BaseModule // pull feed and consume it, which should subscribe to the hub. Worker::add(Worker::PRIORITY_HIGH, 'OnePoll', $contact_id, 'force'); } else { - Worker::add(Worker::PRIORITY_HIGH, 'UpdateContact', $contact_id); + try { + UpdateContact::add(Worker::PRIORITY_HIGH, $contact_id); + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['contact' => $contact, 'callstack' => System::callstack()]); + } } } diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 35348be707..caab13edaf 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -44,6 +44,7 @@ use Friendica\Util\Images; use Friendica\Util\Network; use Friendica\Util\ParseUrl; use Friendica\Util\Proxy; +use Friendica\Worker\UpdateContact; /** * Photo Module @@ -351,8 +352,12 @@ class Photo extends BaseModule Logger::debug('Got return code for avatar', ['return code' => $curlResult->getReturnCode(), 'cid' => $id, 'url' => $contact['url'], 'avatar' => $url]); } if ($update) { - Logger::info('Invalid file, contact update initiated', ['cid' => $id, 'url' => $contact['url'], 'avatar' => $url]); - Worker::add(Worker::PRIORITY_LOW, 'UpdateContact', $id); + try { + UpdateContact::add(Worker::PRIORITY_LOW, $id); + Logger::info('Invalid file, contact update initiated', ['cid' => $id, 'url' => $contact['url'], 'avatar' => $url]); + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['id' => $id, 'contact' => $contact]); + } } else { Logger::info('Invalid file', ['cid' => $id, 'url' => $contact['url'], 'avatar' => $url]); } diff --git a/src/Worker/UpdateContact.php b/src/Worker/UpdateContact.php index 8de3629caf..a20642f0a7 100644 --- a/src/Worker/UpdateContact.php +++ b/src/Worker/UpdateContact.php @@ -22,15 +22,19 @@ namespace Friendica\Worker; use Friendica\Core\Logger; +use Friendica\Core\Worker; use Friendica\Model\Contact; +use Friendica\Network\HTTPException\InternalServerErrorException; class UpdateContact { /** * Update contact data via probe * - * @param int $contact_id Contact ID + * @param int $contact_id Contact ID * @return void + * @throws InternalServerErrorException + * @throws \ImagickException */ public static function execute(int $contact_id) { @@ -38,4 +42,19 @@ class UpdateContact Logger::info('Updated from probe', ['id' => $contact_id, 'success' => $success]); } + + /** + * @param array|int $run_parameters Priority constant or array of options described in Worker::add + * @param int $contact_id + * @return int + * @throws InternalServerErrorException + */ + public static function add($run_parameters, int $contact_id): int + { + if (!$contact_id) { + throw new \InvalidArgumentException('Invalid value provided for contact_id'); + } + + return Worker::add($run_parameters, 'UpdateContact', $contact_id); + } } diff --git a/src/Worker/UpdateContacts.php b/src/Worker/UpdateContacts.php index 881418dc11..138d3e9abd 100644 --- a/src/Worker/UpdateContacts.php +++ b/src/Worker/UpdateContacts.php @@ -63,15 +63,21 @@ class UpdateContacts if (Contact::isLocal($contact['url'])) { continue; } - if ((!empty($contact['gsid']) || !empty($contact['baseurl'])) && GServer::reachable($contact)) { - $stamp = (float)microtime(true); - $success = Contact::updateFromProbe($contact['id']); - Logger::debug('Direct update', ['id' => $contact['id'], 'count' => $count, 'duration' => round((float)microtime(true) - $stamp, 3), 'success' => $success]); - ++$count; - } elseif (Worker::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], 'UpdateContact', $contact['id'])) { - Logger::debug('Update by worker', ['id' => $contact['id'], 'count' => $count]); - ++$count; + + try { + if ((!empty($contact['gsid']) || !empty($contact['baseurl'])) && GServer::reachable($contact)) { + $stamp = (float)microtime(true); + $success = Contact::updateFromProbe($contact['id']); + Logger::debug('Direct update', ['id' => $contact['id'], 'count' => $count, 'duration' => round((float)microtime(true) - $stamp, 3), 'success' => $success]); + ++$count; + } elseif (UpdateContact::add(['priority' => Worker::PRIORITY_LOW, 'dont_fork' => true], $contact['id'])) { + Logger::debug('Update by worker', ['id' => $contact['id'], 'count' => $count]); + ++$count; + } + } catch (\InvalidArgumentException $e) { + Logger::notice($e->getMessage(), ['contact' => $contact]); } + Worker::coolDown(); } DBA::close($contacts);