Code change for associateAttachmentsWithContent

tenants

Well-known member
A minor change, I've seen this method is a bit unsafe to extend/override, it's easy to pass in an empty $tempHash
Doing so will update every attachment in the database (that's not going to be easy to reverse)

can we add a check on the tempHash on this method, something like:

Code:
if(!$tempHash){throw new \InvalidArgumentException("No hash provided for attachment");}

I cant see a reason why you would ever need to pass in an empty $tempHash
Other than some silly addon developer making a mistake (or worse, a malicious user getting to this endpoint)

Original Core Code:

Code:
public function associateAttachmentsWithContent($tempHash, $contentType, $contentId)
    {
        $associated = 0;
        $attachmentFinder = $this->finder(AttachmentFinder::class)
            ->where('temp_hash', $tempHash);
        /** @var Attachment $attachment */
        foreach ($attachmentFinder->fetch() AS $attachment)
        {
            $attachment->content_type = $contentType;
            $attachment->content_id = $contentId;
            $attachment->temp_hash = '';
            $attachment->unassociated = 0;
            $attachment->save();
            $container = $attachment->getContainer();
            $attachment->getHandler()->onAssociation($attachment, $container);
            $associated++;
        }
        return $associated;
   }


You can see the issue with this if $tempHash == "" || null
-- thats you're entire attachment table updated all with the same content_id and content_type (that's going to be annoying to reverse)
 
Last edited:
Upvote 0
Back
Top Bottom