From 63f6efb2dcf6c1e8c6dedd9eb9fb808e1330519a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Roland=20H=C3=A4der?= Date: Fri, 6 Nov 2020 14:44:41 +0100 Subject: [PATCH] Continued: - don't throw exceptions in private methods, they cannot be tested or very difficult there MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Roland Häder --- .../class_PackageRecipientDiscovery.php | 4 +- .../package/class_NetworkPackageHandler.php | 69 +++++++++---------- 2 files changed, 34 insertions(+), 39 deletions(-) diff --git a/application/hub/classes/discovery/recipient/package/class_PackageRecipientDiscovery.php b/application/hub/classes/discovery/recipient/package/class_PackageRecipientDiscovery.php index 7c19d7801..6ae12306a 100644 --- a/application/hub/classes/discovery/recipient/package/class_PackageRecipientDiscovery.php +++ b/application/hub/classes/discovery/recipient/package/class_PackageRecipientDiscovery.php @@ -110,10 +110,10 @@ class PackageRecipientDiscovery extends BaseRecipientDiscovery implements Discov $this->clearRecipients(); // Is the UNL empty but id given? - if ($packageInstance->getRecipientUnl() == '' && $packageInstance->getRecipientId() == '') { + if (empty($packageInstance->getRecipientUnl()) && empty($packageInstance->getRecipientId())) { // Uh, both is empty! throw new InvalidArgumentException('packageInstance has both recipientUnl and recipientId empty.'); - } elseif ($packageInstance->getRecipientUnl() == '' && $packageInstance->getRecipientId() != '') { + } elseif (empty($packageInstance->getRecipientUnl()) && !empty($packageInstance->getRecipientId())) { // Id is set, this might be resolved to an UNL /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('RECIPIENT-DISCOVERY: Resolving recipientId=%s to UNL ...', $packageInstance->getRecipientId())); $recipientUnl = HubTools::resolveSessionIdToUnl($packageInstance->getRecipientId()); diff --git a/application/hub/classes/handler/package/class_NetworkPackageHandler.php b/application/hub/classes/handler/package/class_NetworkPackageHandler.php index fd77d7e57..53a5a8427 100644 --- a/application/hub/classes/handler/package/class_NetworkPackageHandler.php +++ b/application/hub/classes/handler/package/class_NetworkPackageHandler.php @@ -405,18 +405,10 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei * * @param $packageInstance An instance of a DeliverablePackage class * @return $hash Private key's hash - * @throws InvalidPrivateKeyHashException If the private key's hash is not valid - * @throws InvalidArgumentException If $packageInstance does not contain senderId */ private function determineSenderPrivateKeyHash (DeliverablePackage $packageInstance) { - // Is the parameter valid? - /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: packageInstance=%s - CALLED!', $packageInstance->__toString())); - if ($packageInstance->getSenderId() == '') { - // Invalid $packageInstance - throw new InvalidArgumentException('packageInstance does not contain senderId'); - } - // Get DHT instance + /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: packageInstance=%s - CALLED!', $packageInstance->__toString())); $dhtInstance = DhtObjectFactory::createDhtInstance('node'); // Ask DHT for sender's id or address (both session id) @@ -432,12 +424,6 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei return $senderData[NodeDistributedHashTableDatabaseWrapper::DB_COLUMN_PRIVATE_KEY_HASH]; } - // Don't accept empty keys - if (empty($packageInstance->getSenderPrivateKeyHash())) { - // This needs fixing - throw new InvalidPrivateKeyHashException(array($this, $senderData, 'empty hash in decodedData'), BaseHubSystem::EXCEPTION_INVALID_PRIVATE_KEY_HASH); - } - // There is no DHT entry so, accept the hash from decoded data /* NOISY-DEBUG */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: Returning packagheInstance->senderPrivateKeyHash=%s - EXIT!', $packageInstance->getSenderPrivateKeyHash())); return $packageInstance->getSenderPrivateKeyHash(); @@ -499,7 +485,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // This avoids an exception after all packages has failed /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: stackerName=%s is empty - EXIT!', $stackerName)); return; - } // END - if + } // Pop the entry (it should be it) /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: Getting next stack from stackerName=%s ...', $stackerName)); @@ -591,7 +577,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // Skip to next entry /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: Package declared for recipient ' . $currentRecipient); $iteratorInstance->next(); - } // END - while + } /* * The recipient list can be cleaned up here because the package which @@ -704,7 +690,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // This connection is shutting down // @TODO We may want to do somthing more here? return; - } // END - if + } // Sent out package data $helperInstance->sendRawPackageData($packageInstance); @@ -733,7 +719,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // Feature is not enabled /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: Feature "hubcoin_reward" not available, not generating package hash. Returning NULL ...'); return NULL; - } // END - if + } // Fake package instance $packageInstance = ObjectFactory::createObjectByConfiguredName('package_data_class'); @@ -764,7 +750,6 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei * * @param $packageInstance An instance of a DeliverablePackage class * @return $isHashValid Whether the hash is valid - * @throws InvalidArgumentException If $packageInstance contains no senderId */ private function isPackageHashValid (DeliverablePackage $packageInstance) { // Is the feature enabled? @@ -773,9 +758,6 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // Feature is not enabled, so hashes are always valid //* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: Feature "hubcoin_reward" not available, not checking hash. Returning TRUE ...'); return TRUE; - } elseif ($packageInstance->getSenderId() == '') { - // Invalid $packageInstance - throw new InvalidArgumentException('packageInstance does not contain senderId'); } // Check validity @@ -943,7 +925,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // This is not fatal but should be avoided self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: No raw package data waiting declaration, but ' . __METHOD__ . ' has been called!'); return; - } // END - if + } /* * Now there are for sure packages to deliver, so start with the first @@ -975,7 +957,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // This is not fatal but should be avoided self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: No package has been declared, but ' . __METHOD__ . ' has been called!'); return; - } // END - if + } // Get the package instance $packageInstance = $this->getStackInstance()->getNamed(self::STACKER_NAME_DECLARED); @@ -1013,14 +995,14 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // This is not fatal but should be avoided self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: No package is waiting for delivery, but ' . __METHOD__ . ' was called.'); return; - } // END - if + } // Get the package $packageInstance = $this->getStackInstance()->getNamed(self::STACKER_NAME_OUTGOING); // The UNL should be set but not recipientId /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: recipientUnl(%d)=%s,recipientId(%d)=%s', strlen($packageInstance->getRecipientUnl()), $packageInstance->getRecipientUnl(), strlen($packageInstance->getRecipientId()), $packageInstance->getRecipientId())); - if ($packageInstance->getRecipientUnl() != '' && $packageInstance->getRecipientId() == '') { + if (!empty($packageInstance->getRecipientUnl()) && empty($packageInstance->getRecipientId())) { // Need to resolve UNL -> session id ("recipient id"), so prepare UNL instance /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: recipientUnl=%s is being solved into a session id ...', $packageInstance->getRecipientUnl())); $locatorInstance = UniversalNodeLocatorFactory::createUnlInstanceFromString($packageInstance->getRecipientUnl()); @@ -1083,7 +1065,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei if (!$socketInstance->writeBufferToSocketByArray($encodedDataArray, $sentBytes)) { // Something bad happened while writing to socket $socketInstance->handleSocketError(__METHOD__, __LINE__); - } // END - if + } // Debug message /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: sentBytes[%s]=%d', gettype($sentBytes), $sentBytes)); @@ -1174,7 +1156,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei if (!$this->isRawDataPending()) { // Should not be invoked anymore throw new BadMethodCallException('No incoming decoded data on stack but method was called.'); - } // END - if + } // "Pop" the next entry (the same array again) from the stack /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: Stacker size is %d entries.', $this->getStackInstance()->getStackCount(self::STACKER_NAME_DECODED_INCOMING))); @@ -1190,7 +1172,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei // It is there and should be removed /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: Raw data contains CHUNK_SEPARATOR=%s - Removing ...', PackageFragmenter::CHUNK_SEPARATOR)); $packageInstance->setRawData(substr($packageInstance->getRawData(), 0, -1)); - } // END - if + } // This package is "handled" and can be pushed on the next stack /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: Pushing %d(%s) bytes to stack %s ...', strlen($packageInstance->getRawData()), $packageInstance->getRawData(), self::STACKER_NAME_DECODED_HANDLED)); @@ -1356,7 +1338,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei if ($packageInstance->getStatus() != self::PACKAGE_STATUS_FAILED) { // Not failed! throw new UnexpectedPackageStatusException(array($this, $packageInstance, self::PACKAGE_STATUS_FAILED), self::EXCEPTION_UNEXPECTED_PACKAGE_STATUS); - } // END - if + } // Remove this entry $this->getStackInstance()->popNamed(self::STACKER_NAME_DECLARED); @@ -1425,7 +1407,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei public function handleReceivedPackageInstance (DeliverablePackage $packageInstance) { // Is the package content set? /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput(sprintf('NETWORK-PACKAGE-HANDLER: packageInstance=%s - CALLED!', $packageInstance->__toString())); - if ($packageInstance->getPackageContent() == '') { + if (empty($packageInstance->getPackageContent())) { // Package content is not set throw new InvalidArgumentException(sprintf('Package with receipientId=%s,recipientType=%s has empty packageContent', $packageInstance->getRecipientId(), @@ -1498,21 +1480,34 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei * Handles newly arrived messages * * @return void + * @throws BadMethodCallException If no new message has arrived + * @throws InvalidArgumentException If $packageInstance contains no senderId + * @throws InvalidPrivateKeyHashException If the private key-hash is empty * @todo Implement verification of all sent tags here? */ public function handleNewlyArrivedMessage () { // Make sure there is at least one message /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: CALLED!'); - assert($this->isNewMessageArrived()); + if (!$this->isNewMessageArrived()) { + // Bad method call + throw new BadMethodCallException('No new message arrived but this method has been called.'); + } // Get it from the stacker, it is the full array with the decoded message $packageInstance = $this->getStackInstance()->popNamed(self::STACKER_NAME_NEW_MESSAGE); // Generate the hash of comparing it - if (!$this->isPackageHashValid($packageInstance)) { + /* NOISY-DEBUG: */ self::createDebugInstance(__CLASS__, __LINE__)->debugOutput('NETWORK-PACKAGE-HANDLER: packageInstance=%s,senderId=%s,senderPrivateKeyHash=%s', $packageInstance->__toString(), $packageInstance->getSenderId(), $packageInstance->getSenderPrivateKeyHash())); + if (empty($packageInstance->getSenderId())) { + // Invalid $packageInstance + throw new InvalidArgumentException('packageInstance does not contain senderId'); + } elseif (empty($packageInstance->getSenderPrivateKeyHash())) { + // This needs fixing + throw new InvalidPrivateKeyHashException(array($this, $senderData, 'empty hash in decodedData'), BaseHubSystem::EXCEPTION_INVALID_PRIVATE_KEY_HASH); + } elseif (!$this->isPackageHashValid($packageInstance)) { // Is not valid, so throw an exception here exit(__METHOD__ . ':INVALID HASH! UNDER CONSTRUCTION!' . chr(10)); - } // END - if + } // Now get a filter chain back from factory with given tags array $chainInstance = PackageFilterChainFactory::createChainByTagsArray($packageInstance->getContentHash()); @@ -1596,7 +1591,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei *may be invalid. */ return; - } // END - if + } // Copy node id $messageData[self::MESSAGE_ARRAY_DATA_NODE_ID] = $messageData[self::MESSAGE_ARRAY_DATA][self::MESSAGE_ARRAY_DATA_NODE_ID]; @@ -1616,7 +1611,7 @@ class NetworkPackageHandler extends BaseHubHandler implements Deliverable, Recei * endless loop of mining. */ return; - } // END - if + } // Also remove tags as the miner don't need this. unset($messageData[self::MESSAGE_ARRAY_TAGS]); -- 2.39.5