UploadField setAllowedExtension ignored

[ghost]

Hi all,

On SS 4 I'm trying to set these extensions on UploadField like as I always did on version 3, like this:

<?php
// Definizione Namespace
use SilverStripe\Security\Permission;
use SilverStripe\ORM\DataExtension;
use SilverStripe\Forms\FieldList;
use SilverStripe\Assets\Folder;
use SilverStripe\Assets\File;
use SilverStripe\AssetAdmin\Forms\UploadField;
use SilverStripe\Forms\GridField\GridField;
use SilverStripe\Forms\GridField\GridFieldConfig_RecordViewer;

/**
 * Classe Utente - Estensione
 */
class UtenteExtension extends DataExtension
{
    // Dichiarazione Proprietà
    private static $many_many = [
        'AllegatiDownload' => File::class
    ];
    private static $owns = [
        'AllegatiDownload'
    ];

    /**
     * Metodo aggiornamento campi Back-End
     * Setter
     * @param FieldList $fields Campi Form
     * @return void
     */
    public function updateCMSFields(FieldList $fields)
    {
        $cartellaDownload = 'pazienti/'. strtolower($this->owner->Surname) .'/downloads';
        $dimensioneFile = 20 * 1024 * 1024; // 20 MB

        // Controllo permessi
        if (Permission::checkMember($this->owner->ID, 'UTENTE')) {
            $fields->removeByName('AllegatiDownload');
            $fields->addFieldToTab('Root.Downloads', $downloadFileUtente = new UploadField('AllegatiDownload', 'Files utente scaricabili - 20 MB max.)'));
            $downloadFileUtente->getValidator()->setAllowedExtensions(array('ply', 'obj', 'mtl', 'psd'));
            $downloadFileUtente->getValidator()->setAllowedMaxFileSize($dimensioneFile);

            Folder::find_or_make($cartellaDownload);

            $downloadFileUtente->setFolderName($cartellaDownload);
            $downloadFileUtente->setIsMultiUpload(true);
        }
    }

    /**
     * Metodo gestione eventi post-salvataggio
     * Setter
     */
    public function onAfterWrite()
    {
        if ($this->owner->AllegatiDownloadID) {
            $this->owner->AllegatiDownload()->publishSingle();
        }
    }
}

Trying the module on back-end seems like the directive being totally ignored, in fact none of those extensions results allowed. Here's a screen about it:

PHP is already set to manage files up to 20MB so that's excluded. As you can see framework is explicitly throwing the extension error. Same is reported on console log. Am I missing a passage in order to allow them? Is changed something about it in this new version? Or any way to force them maybe?

Thanks in advance for support.

[ghost]

As suggested here , in order to achieve the setting I applied the global configuration like this:

SilverStripe\Assets\File:
  allowed_extensions:
    - 'doc'
    - 'docx'
    - 'xls'
    - 'xlsx'
    - 'pdf'
    - 'jpg'
    - 'jpeg'
    - 'png'
    - 'gif'
    - 'ply'

By setting extensions via yaml, the rules work, but not in the above mentioned way.

[chillu]

Since we provide an Upload_Validator->setAllowedExtensions() API, this should be possible without the YAML workaround you described - adding type/bug and impact/high since file extension restriction can be important depending on context.

[robbieaverill]

At a quick look, it seems that DBFile has its own set of allowed_extensions, so while setting the allowed extensions on the upload field works, it fails when the underlying DBFile is written

[robbieaverill]

Still reproducible on SS 4.3.x-dev. I'm going to have a look at this now.

[robbieaverill]

So DBFile::validate() checks globally configured allowed extensions, it doesn't check any instance specific rules. The tricky thing about this is that even if you pass Upload_Validator down to the underlying DBFile so it can use it for validation then it allows you to upload the file, but it fails when you publish it because the abstracted Form -> validate() logic calls File::validate() which does it again without the context of the Upload_Validator that you create in updateCMSFields.

The validator instance is not persisted at all, it's only created when you are generating CMS fields. The error is generated from DataObject::write() which is triggered through a ChangeSet publication, and subsequently calls ::validate().

One option is to disable file extension (and probably max size) validation for records that are already in the DB, i.e. when publishing. This would mean that the burden would be entirely on the form field to handle whether the file is initially allowed to be uploaded, and once it's in the system we'd skip re-validation of it. We could also limit the scope of what we ignore here, if we decided to go down this road, to anything that is already covered by Upload_Validator during the initial file upload process. Of course, this assumes that you're using an Upload_Validator in the first place.

I've bumped the impact rating because this issue requires in-depth SilverStripe knowledge to resolve.

Edit: by the way, I think the difference here between SilverStripe 3.x and 4.x is that in 4.x files are versioned, so while we can adjust the code to pass the validator down for the initial write, it doesn't exist any more for subsequent writes on publish - the latter didn't exist in SilverStripe 3.

[tractorcow]

maybe setAllowedExtensions() should validate it's args vs the core internal allowed list?

[robbieaverill]

@tractorcow do you mean that if you try to $upload->getValidator()->setAllowedExtensions(['php']) it would fail because File.allowed_extensions doesn't contain php? That would effectively mean we'd be saying "you can reduce the number of extensions your upload can contain past those defined in File.allowed_extensions, but you can't increase it"

This functionality is broken at the moment, so that'd be better than nothing, but we'd need to make sure that's what we want to do going forward.

[noizyboy]

Any progress or potential workarounds on this one? (That don't involve global-level yaml updates?)

[jackkemmish]

Any movement on this? I'm trying to implemenet this and it's not validating anything. Would be ideal to do $upload->getValidator()->setAllowedExtensions(['php']) for a case by case basis.