Fixed unassociatedAttachmentLimit calculated incorrectly

[mattrogowski]

Affected version

2.3.2

Very minor issue but the unassociatedAttachmentLimit config value actually accepts 1 less than the limit. Easier to test if you set $config['unassociatedAttachmentLimit'] = 2;, you get the error after uploading 1 attachment instead of 2.

src/XF/Attachment/Manipulator.php:140:

$allowed = ($uploaded < $unassociatedLimit);

just needs to be:

$allowed = ($uploaded <= $unassociatedLimit);

Then it accepts 2 and rejects a 3rd.

Strangely the check above works with <, can't immediately work out why one check works and the other doesn't.

 

[mattrogowski]

Actually seems to be that the second $uploaded value is wrong. If I dump that value out and upload 3 attachments that value comes out as 4.





The second $uploaded does $unassociatedAttachmentCount + count($this->newAttachments); which seems to be counting them twice (which makes sense, the new attachments will be unassociated), see how the first one goes 0, 1, 2 and the second goes 0, 2, 4.

Probably never come up because the default limit is 100, so unless you lower it or upload a huge amount of attachments at once you'd never hit the issue. It means though that you can only upload 50 before it rejects them, instead of the actual limit of 100.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.3.5).

Change log:

Fix unassociated attachment limit checks

There may be a delay before changes are rolled out to the XenForo Community.