From 62a9139d149dc5d597f57411ceb8c1f740d37754 Mon Sep 17 00:00:00 2001 From: Deon George Date: Thu, 11 Jul 2024 23:33:17 +1000 Subject: [PATCH] Fix packet processing issue - we now find recent deleted address when creatingFTN, fix netmail processing with points, fix processing badly address netmails --- app/Classes/FTN.php | 4 +- app/Classes/FTN/Message.php | 5 +- app/Classes/FTN/Packet.php | 26 +++++--- app/Jobs/PacketProcess.php | 8 +++ app/Models/Address.php | 10 ++- app/Models/Echomail.php | 10 +-- app/Models/Netmail.php | 2 +- .../Netmails/EchomailBadAddress.php | 3 +- .../Netmails/NetmailBadAddress.php | 65 +++++++++++++++++++ app/Traits/ParseAddresses.php | 2 +- 10 files changed, 110 insertions(+), 25 deletions(-) create mode 100644 app/Notifications/Netmails/NetmailBadAddress.php diff --git a/app/Classes/FTN.php b/app/Classes/FTN.php index 9dc8e25..040f67a 100644 --- a/app/Classes/FTN.php +++ b/app/Classes/FTN.php @@ -17,7 +17,7 @@ abstract class FTN $this->fn, $this->ff, $this->fp, - ).($this->zone ? sprintf('@%s',$this->zone->domain->name) : ''); + ).((isset($this->zone) && $this->zone) ? sprintf('@%s',$this->zone->domain->name) : ''); case 'tftn_t': return sprintf('%d:%d/%d.%d', @@ -25,7 +25,7 @@ abstract class FTN $this->tn, $this->tf, $this->tp, - ).($this->zone ? sprintf('@%s',$this->zone->domain->name) : ''); + ).((isset($this->zone) && $this->zone) ? sprintf('@%s',$this->zone->domain->name) : ''); case 'fftn': return Address::findFTN($this->fftn_t); diff --git a/app/Classes/FTN/Message.php b/app/Classes/FTN/Message.php index b9a6389..de89190 100644 --- a/app/Classes/FTN/Message.php +++ b/app/Classes/FTN/Message.php @@ -306,7 +306,7 @@ class Message extends FTNBase case 'fz': return (int)Arr::get($this->src,'z'); case 'fn': return (int)($x=$this->src) ? Arr::get($x,'n') : Arr::get($this->header,'onet'); case 'ff': return (int)($x=$this->src) ? Arr::get($x,'f') : Arr::get($this->header,'onode'); - case 'fp': return (int)$this->mo->kludges->get('FMPT:') ?: Arr::get($this->src,'p',Arr::get($this->header,'opoint',0)); + case 'fp': return (int)$this->mo->kludges->get('FMPT') ?: Arr::get($this->src,'p',Arr::get($this->header,'opoint',0)); case 'fd': return Arr::get($this->src,'d'); case 'fzone': @@ -323,6 +323,7 @@ class Message extends FTNBase return Zone::where('zone_id',$this->fz) ->where('default',TRUE) ->single(); + case 'fdomain': // We'll use the zone's domain if this method class was called with a zone if ($this->zone && (($this->zone->domain->name === Arr::get($this->src,'d')) || ! Arr::get($this->src,'d'))) @@ -340,7 +341,7 @@ class Message extends FTNBase case 'tz': return (int)Arr::get($this->isEchomail() ? $this->src : $this->dst,'z'); case 'tn': return (int)Arr::get($this->header,'dnet'); case 'tf': return (int)Arr::get($this->header,'dnode'); - case 'tp': return (int)$this->mo->kludges->get('TOPT:') ?: Arr::get($this->header,'dpoint',0); + case 'tp': return (int)$this->mo->kludges->get('TOPT') ?: Arr::get($this->header,'dpoint',0); case 'tzone': // Use the zone if this class was called with it. diff --git a/app/Classes/FTN/Packet.php b/app/Classes/FTN/Packet.php index 9e83c50..2468395 100644 --- a/app/Classes/FTN/Packet.php +++ b/app/Classes/FTN/Packet.php @@ -3,7 +3,6 @@ namespace App\Classes\FTN; use Carbon\Carbon; -use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; @@ -13,7 +12,7 @@ use Symfony\Component\HttpFoundation\File\File; use App\Classes\FTN as FTNBase; use App\Exceptions\InvalidPacketException; use App\Models\{Address,Domain,Echomail,Netmail,Software,System,Zone}; -use App\Notifications\Netmails\EchomailBadAddress; +use App\Notifications\Netmails\{EchomailBadAddress,NetmailBadAddress}; /** * Represents a Fidonet Packet, that contains an array of messages. @@ -139,7 +138,7 @@ abstract class Packet extends FTNBase implements \Iterator, \Countable } else throw new InvalidPacketException('Not a valid packet, not EOP or SOM:'.bin2hex($x)); - Log::info(sprintf('%s:- Packet [%s] is a [%s] packet, dated [%s]',self::LOGKEY,$o->name,get_class($o),$o->date)); + Log::info(sprintf('%s:- Packet [%s] is a [%s] packet',self::LOGKEY,$o->name,get_class($o))); // Work out the packet zone if ($o->fz && ($o->fd || $domain)) { @@ -159,6 +158,8 @@ abstract class Packet extends FTNBase implements \Iterator, \Countable ->singleOrFail(); } + Log::info(sprintf('%s:- Packet Dated [%s] from [%s] to [%s]',self::LOGKEY,$o->date,$o->fftn_t,$o->tftn_t)); + $message = ''; // Current message we are building $msgbuf = ''; $leader = Message::header_len()+strlen(self::PACKED_MSG_LEAD); @@ -384,10 +385,11 @@ abstract class Packet extends FTNBase implements \Iterator, \Countable // If the messages is not for the right zone, we'll ignore it if ($msg->errors->has('invalid-zone')) { - Log::alert(sprintf('%s:! Message [%s] is from an invalid zone [%s], packet is from [%s] - ignoring it',self::LOGKEY,$msg->msgid,$msg->fftn->zone->zone_id,$this->fftn->zone->zone_id)); + Log::alert(sprintf('%s:! Message [%s] is from an invalid zone [%s], packet is from [%s] - ignoring it',self::LOGKEY,$msg->msgid,$msg->set_fftn,$this->fz)); + // @todo $msg might not be echomail if (! $msg->kludges->get('RESCANNED')) - Notification::route('netmail',$this->fftn)->notify(new EchomailBadAddress($msg)); + Notification::route('netmail',$this->fftn)->notify(($msg instanceof Echomail) ? new EchomailBadAddress($msg) : new NetmailBadAddress($msg)); return; } @@ -396,7 +398,7 @@ abstract class Packet extends FTNBase implements \Iterator, \Countable if ($msg->errors->has('from') && $this->fftn && $this->fftn->zone_id) { Log::debug(sprintf('%s:^ From address [%s] doesnt exist, it needs to be created',self::LOGKEY,$msg->set->get('set_fftn'))); - $ao = Address::findFTN($msg->set->get('set_fftn'),TRUE); + $ao = Address::findFTN($msg->set->get('set_fftn'),TRUE,TRUE); if ($ao?->exists && ($ao->zone?->domain_id !== $this->fftn->zone->domain_id)) { Log::alert(sprintf('%s:! From address [%s] domain [%d] doesnt match packet domain [%d]?',self::LOGKEY,$msg->set->get('set_fftn'),$ao->zone?->domain_id,$this->fftn->zone->domain_id)); @@ -407,17 +409,20 @@ abstract class Packet extends FTNBase implements \Iterator, \Countable if (! $ao) { $so = System::createUnknownSystem(); $ao = Address::createFTN($msg->set->get('set_fftn'),$so); + + Log::alert(sprintf('%s:- From FTN [%s] is not defined, created new entry for (%d)',self::LOGKEY,$msg->set->get('set_fftn'),$ao->id)); } $msg->fftn_id = $ao->id; - Log::alert(sprintf('%s:- From FTN [%s] is not defined, created new entry for (%d)',self::LOGKEY,$msg->set->get('set_fftn'),$ao->id)); + $msg->errors->forget('from'); + $msg->errors->forget('fftn_id'); } // If the $msg->tftn doesnt exist, we'll need to create it if ($msg->errors->has('to') && $this->tftn && $this->tftn->zone_id) { Log::debug(sprintf('%s:^ To address [%s] doesnt exist, it needs to be created',self::LOGKEY,$msg->set->get('set_tftn'))); - $ao = Address::findFTN($msg->set->get('set_tftn'),TRUE); + $ao = Address::findFTN($msg->set->get('set_tftn'),TRUE,TRUE); if ($ao?->exists && ($ao->zone?->domain_id !== $this->tftn->zone->domain_id)) { Log::alert(sprintf('%s:! To address [%s] domain [%d] doesnt match packet domain [%d]?',self::LOGKEY,$msg->set->get('set_tftn'),$ao->zone?->domain_id,$this->fftn->zone->domain_id)); @@ -428,10 +433,13 @@ abstract class Packet extends FTNBase implements \Iterator, \Countable if (! $ao) { $so = System::createUnknownSystem(); $ao = Address::createFTN($msg->set->get('set_fftn'),$so); + + Log::alert(sprintf('%s:- To FTN [%s] is not defined, created new entry for (%d)',self::LOGKEY,$msg->set->get('set_tftn'),$ao->id)); } $msg->tftn_id = $ao->id; - Log::alert(sprintf('%s:- To FTN [%s] is not defined, created new entry for (%d)',self::LOGKEY,$msg->set->get('set_tftn'),$ao->id)); + $msg->errors->forget('to'); + $msg->errors->forget('tftn_id'); } // If there is no fftn, then its from a system that we dont know about diff --git a/app/Jobs/PacketProcess.php b/app/Jobs/PacketProcess.php index b1ddc1a..afa8520 100644 --- a/app/Jobs/PacketProcess.php +++ b/app/Jobs/PacketProcess.php @@ -86,6 +86,14 @@ class PacketProcess implements ShouldQueue break; } + // If we dont have the tftn in the DB, then packet cannot be to us + if (! $pkt->tftn) { + Log::error(sprintf('%s:! Packet [%s] is from a system [%s] we dont know about?',self::LOGKEY,$this->filename,$pkt->tftn_t)); + + // @todo Notification::route('netmail',$pkt->fftn)->notify(new UnexpectedPacketToUs($this->filename)); + break; + } + // Check the packet is to our address, if not we'll reject it. if (! our_address($pkt->tftn->zone->domain)->contains($pkt->tftn)) { Log::error(sprintf('%s:! Packet [%s] is not to our address? [%s]',self::LOGKEY,$this->filename,$pkt->tftn->ftn)); diff --git a/app/Models/Address.php b/app/Models/Address.php index 7bdee9b..a1aaffc 100644 --- a/app/Models/Address.php +++ b/app/Models/Address.php @@ -175,10 +175,11 @@ class Address extends Model * * @param string $address * @param bool $trashed + * @param bool $recent * @return Address|null * @throws \Exception */ - public static function findFTN(string $address,bool $trashed=FALSE): ?self + public static function findFTN(string $address,bool $trashed=FALSE,bool $recent=FALSE): ?self { $ftn = self::parseFTN($address); $o = NULL; @@ -187,8 +188,11 @@ class Address extends Model ->select('addresses.*') ->join('zones',['zones.id'=>'addresses.zone_id']) ->join('domains',['domains.id'=>'zones.domain_id']) - ->when($trashed,function($query) { - $query->withTrashed(); + ->when($trashed,function($query) use ($recent) { + return $query->withTrashed() + ->orderBy('updated_at','DESC') + ->when($recent,fn($query)=>$query->where(fn($query)=>$query + ->where('deleted_at','>=',Carbon::now()->subMonth())->orWhereNull('deleted_at'))); },function($query) { $query->active(); }) diff --git a/app/Models/Echomail.php b/app/Models/Echomail.php index 2d1dbb6..3ead694 100644 --- a/app/Models/Echomail.php +++ b/app/Models/Echomail.php @@ -150,7 +150,7 @@ final class Echomail extends Model implements Packet // Make sure our origin contains our FTN $m = []; if ((preg_match('#^(.*)\s+\(([0-9]+:[0-9]+/[0-9]+.*)\)+\s*$#',$model->set_origin,$m)) - && (Address::findFTN(sprintf('%s@%s',$m[2],$model->fftn->domain->name))?->id === $model->fftn_id)) + && (Address::findFTN(sprintf('%s@%s',$m[2],$model->fftn->domain->name),TRUE,TRUE)?->id === $model->fftn_id)) { $x = Origin::where('value',utf8_encode($m[1]))->single(); @@ -184,13 +184,13 @@ final class Echomail extends Model implements Packet Log::debug(sprintf('%s:^ Message [%d] from point address is [%d]',self::LOGKEY,$model->id,$model->fftn->point_id)); // Make sure our sender is first in the path - if ((! $model->isFlagSet(Message::FLAG_LOCAL)) && (! $path->contains($model->fftn_id))) { + if (($model->fftn->point_id === 0) && (! $model->isFlagSet(Message::FLAG_LOCAL)) && (! $path->contains($model->fftn_id))) { Log::alert(sprintf('%s:? Echomail adding sender to start of PATH [%s].',self::LOGKEY,$model->fftn_id)); $path->prepend($model->fftn_id); } // Make sure our pktsrc is last in the path - if ($model->set->has('set_sender') && (! $path->contains($model->set->get('set_sender')->id))) { + if ($model->set->has('set_sender') && (! $path->contains($model->set->get('set_sender')->id)) && ($model->set->get('set_sender')->point_id === 0)) { Log::alert(sprintf('%s:? Echomail adding pktsrc to end of PATH [%s].',self::LOGKEY,$model->set->get('set_sender')->ftn)); $path->push($model->set->get('set_sender')->id); } @@ -215,13 +215,13 @@ final class Echomail extends Model implements Packet $seenby = self::parseAddresses('seenby',$model->set->get('set_seenby'),$model->fftn->zone,$rogue); // Make sure our sender is in the seenby - if ((! $model->isFlagSet(Message::FLAG_LOCAL)) && (! $seenby->contains($model->fftn_id))) { + if (($model->fftn->point_id === 0) && (! $model->isFlagSet(Message::FLAG_LOCAL)) && (! $seenby->contains($model->fftn_id))) { Log::alert(sprintf('%s:? Echomail adding sender to SEENBY [%s].',self::LOGKEY,$model->fftn_id)); $seenby->push($model->fftn_id); } // Make sure our pktsrc is in the seenby - if ($model->set->has('set_sender') && (! $seenby->contains($model->set->get('set_sender')->id))) { + if ($model->set->has('set_sender') && (! $seenby->contains($model->set->get('set_sender')->id)) && ($model->set->get('set_sender')->point_id === 0)) { Log::alert(sprintf('%s:? Echomail adding pktsrc to SEENBY [%s].',self::LOGKEY,$model->set->get('set_sender')->ftn)); $seenby->push($model->set->get('set_sender')->id); } diff --git a/app/Models/Netmail.php b/app/Models/Netmail.php index a929b9f..75c1c4f 100644 --- a/app/Models/Netmail.php +++ b/app/Models/Netmail.php @@ -142,7 +142,7 @@ final class Netmail extends Model implements Packet // Make sure our origin contains our FTN $m = []; if ((preg_match('#^(.*)\s+\(([0-9]+:[0-9]+/[0-9]+.*)\)+\s*$#',$model->set_origin,$m)) - && (Address::findFTN(sprintf('%s@%s',$m[2],$model->fftn->domain->name))?->id === $model->fftn_id)) + && (Address::findFTN(sprintf('%s@%s',$m[2],$model->fftn->domain->name),TRUE,TRUE)?->id === $model->fftn_id)) { $x = Origin::where('value',utf8_encode($m[1]))->single(); diff --git a/app/Notifications/Netmails/EchomailBadAddress.php b/app/Notifications/Netmails/EchomailBadAddress.php index a558448..86a5710 100644 --- a/app/Notifications/Netmails/EchomailBadAddress.php +++ b/app/Notifications/Netmails/EchomailBadAddress.php @@ -2,7 +2,6 @@ namespace App\Notifications\Netmails; -use Carbon\Carbon; use Illuminate\Support\Facades\Log; use App\Notifications\Netmails; @@ -50,7 +49,7 @@ class EchomailBadAddress extends Netmails $msg->addText($this->sourceSummary($this->mo)."\r\r"); - $msg->addText(sprintf("The address in this echomail [%s] is the wrong address for this domain [%s].\r\r",$this->mo->fftn,$ao->zone->domain->name)); + $msg->addText(sprintf("The address in this echomail [%s] is the wrong address for the domain [%s].\r\r",$this->mo->fftn,$ao->zone->domain->name)); $msg->addText("This echomail has been rejected and not stored here - so no downstream nodes will receive it. If you think this is a mistake, please let me know.\r\r"); diff --git a/app/Notifications/Netmails/NetmailBadAddress.php b/app/Notifications/Netmails/NetmailBadAddress.php new file mode 100644 index 0000000..84624a1 --- /dev/null +++ b/app/Notifications/Netmails/NetmailBadAddress.php @@ -0,0 +1,65 @@ +mo = $mo; + } + + /** + * Get the mail representation of the notification. + * + * @param mixed $notifiable + * @return Netmail + * @throws \Exception + */ + public function toNetmail(object $notifiable): Netmail + { + $o = $this->setupNetmail($notifiable); + $ao = $notifiable->routeNotificationFor(static::via); + + Log::info(sprintf('%s:+ Creating NETMAIL BAD ADDRESS netmail to [%s]',self::LOGKEY,$ao->ftn)); + + $o->subject = sprintf('Bad address in netmail [%s]',$this->mo->msgid); + + // Message + $msg = $this->page(FALSE,'badmsg'); + + $msg->addText($this->sourceSummary($this->mo)."\r\r"); + + $msg->addText(sprintf("The address in this netmail [%s] is the wrong address for the domain [%s].\r\r",$this->mo->set_fftn,$ao->zone->domain->name)); + + $msg->addText("This netmail has been rejected and not stored here - so no downstream node will receive it. If you think this is a mistake, please let me know.\r\r"); + + $msg->addText($this->message_path($this->mo)); + + $o->msg = $msg->render(); + $o->set_tagline = 'I enjoyed reading your message, even though nobody else will get it :)'; + + $o->save(); + + return $o; + } +} \ No newline at end of file diff --git a/app/Traits/ParseAddresses.php b/app/Traits/ParseAddresses.php index 429d724..1884254 100644 --- a/app/Traits/ParseAddresses.php +++ b/app/Traits/ParseAddresses.php @@ -37,7 +37,7 @@ trait ParseAddresses // If domain should be flattened, look for node regardless of zone (within the list of zones for the domain) $ao = ($zone->domain->flatten) ? Address::findZone($zone->domain,$net&Address::ADDRESS_FIELD_MAX,$node&Address::ADDRESS_FIELD_MAX,0) - : Address::findFTN(sprintf('%d:%d/%d@%s',$zone->zone_id,$net&Address::ADDRESS_FIELD_MAX,$node&Address::ADDRESS_FIELD_MAX,$zone->domain->name)); + : Address::findFTN(sprintf('%d:%d/%d@%s',$zone->zone_id,$net&Address::ADDRESS_FIELD_MAX,$node&Address::ADDRESS_FIELD_MAX,$zone->domain->name),TRUE,TRUE); switch ($type) { case 'seenby':