WoltLab / com.woltlab.wcf.conversation

User Conversation System
GNU Lesser General Public License v2.1
13 stars 13 forks source link

Prevent method calls on `null` in ConversationMessageAttachmentObjectType::canDownload() #157

Closed TimWolla closed 3 years ago

TimWolla commented 3 years ago

This may happen for attachments where the corresponding conversation / conversation message no longer exists.


Shall we add an update script that cleans up orphaned attachments?

dtdesign commented 3 years ago

Shall we add an update script that cleans up orphaned attachments?

Are attachments not cleaned up in general or only in case of errors? That is, does it affect a potentially large amount of files? If yes, I would postpone this to the next upgrade (5.4 -> 5.5) instead of running it now.

TimWolla commented 3 years ago

It appears that cleanup might just be missing in cases of error / bugs. Checking WoltLab.com this appears to affect a low 2-digit number of attachments. I would be fine with delaying this until the next larger upgrade nonetheless. It's no severe reliability issue and it might also affect the other apps.

SELECT *
FROM `wcf1_attachment`
WHERE objecttypeid IN
    (SELECT objecttypeid
     FROM wcf1_object_type
     WHERE objecttype = 'com.woltlab.wcf.conversation.message'
       AND definitionid IN
         (SELECT definitionid
          FROM wcf1_object_type_definition
          WHERE definitionname = 'com.woltlab.wcf.attachment.objectType' ))
  AND objectid NOT IN
    (SELECT messageid
     FROM wcf1_conversation_message); 
SoftCreatR commented 3 years ago

In my case, it affects 22 out of 211.329 attachments.