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:
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:
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)
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