UploadProductImages does not error check the upload

[reetp]

When we upload a file we should really error check the upload completes before running anything else like insertattachment

So roughly this, but needs checking that error is not = 0 and an exit so if the file is not uploaded we don't run insertToAttachment etc.

Not quite sure the best way to do it, but at least it is noted here.

foreach ($fileData as $imageDetail) {
    $_FILES = array();
    $filepath = $root_directory . 'cache/' . basename($imageDetail['name']);

• file_put_contents($filepath, base64_decode($imageDetail['content']));
• $error = file_put_contents($filepath, base64_decode($imageDetail['content']));

// This shouldn't run from here down if the file is not uploaded

    $_FILES['imagename'] = array(
        'name' => $imageDetail['name'],
        'type' => finfo_file($finfo, $filepath),
        'tmp_name' => $filepath,
        'error' => 0,
        'size' => filesize($filepath),
    );
    $product->insertIntoAttachment($crmid, 'Products', true);
    unlink($filepath);
    $myResult['FileName'][] = $imageDetail['name'];
    $newnumimages++;
    if ($newnumimages >= $maximages) {
        break;
    }
}
$rsimg = $adb->pquery('select count(*) as cnt
    from vtiger_attachments
    inner join vtiger_crmentity on vtiger_crmentity.crmid = vtiger_attachments.attachmentsid
    inner join vtiger_seattachmentsrel on vtiger_attachments.attachmentsid=vtiger_seattachmentsrel.attachmentsid
    where deleted=0 and vtiger_crmentity.setype LIKE "Products Image" and vtiger_seattachmentsrel.crmid=?', array($crmid));
$finalnumimages = $adb->query_result($rsimg, 0, 'cnt');

// Check that final total of images equals the start number plus the file count
if ($finalnumimages == ($numimages + count($fileData))) {
    $myResult['Error'] = '0';
    $myResult['ImagesAtStart'] = $numimages;
    $myResult['NumberToAdd'] = count($fileData);
    $myResult['ImagesAtFinish'] = $newnumimages;
    $myResult['MaxImages'] = $maximages;
} else { // If not return the images actually inserted and an error code
    $myResult['Error'] = '1';
    $myResult['ErrorStr'] = 'Maximum number of images has been reached';
    $myResult['ImagesAtStart'] = $numimages;
    $myResult['NumberToAdd'] = count($fileData);
    $myResult['ImagesAtFinish'] = $newnumimages;
    $myResult['MaxImages'] = $maximages;
}

[reetp]

Here's a funny thing I have just seen. There are two insertIntoAttachment functions. One in Products.php and one in CRMEntity.php

Is that necessary ?

[joebordes]

I'll have a look at adding that check when I visit the security enhancements in image upload.

The insertintoattachment method on products is special because it has to support the special carusel field on that module. That is what the first loop is for, at the end it calls the parent method to support other normal image fields.