Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File Validator: Field 'file' Cannot be empty #15051

Closed
RayHughes opened this issue May 24, 2020 · 8 comments · Fixed by #15432
Closed

File Validator: Field 'file' Cannot be empty #15051

RayHughes opened this issue May 24, 2020 · 8 comments · Fixed by #15432
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release new feature request Planned Feature or New Feature Request

Comments

@RayHughes
Copy link

RayHughes commented May 24, 2020

When using one of the File Validators, a message is set stating that the "field cannot be empty" when it is in fact set. The only work around with this is to write a call back validator or use the $_FILES global.

This feels wrong as its not compatible with an array of Phalcon\Http|Request\File set by $this->request->getUploadedFiles(true)

@ruudboon ruudboon added 4.1.0 bug A bug report need script to reproduce Script is required to reproduce the issue status: unverified Unverified labels May 31, 2020
@ruudboon
Copy link
Member

@RayHughes Can you provide us with a code example?

@RayHughes
Copy link
Author

Here is the code, I pulled it from its classes etc.

CommandHandler

public function handle(User $user, array $fileData): void
{
    $userAvatarUploadValidator = new UserAvatarUploadValidator();
    $isInvalid = $userAvatarUploadValidator->validate($fileData);

    if ($isInvalid->valid()) {
        $message = $isInvalid->current();

        throw new InvalidArgumentException($message->getMessage());
    }
}

Call

$fileData = $this->request->getUploadedFiles(true, true);

handle($user, $fileData);

Validator

class UserAvatarUploadValidator extends Validation
{
    public function initialize(): void
    {
        $this->add(
            'avatarFile',
            new PresenceOf(
                [
                    'message' => 'Avatar must be provided',
                ]
            )
        );

        $this->add(
            'avatarFile',
            new File(
                [
                    'maxSize' => '5M',
                    'messageSize' => 'Avatar file cannot be larger than 5M',
                    'allowedTypes' => [
                        'image/jpeg',
                        'image/png',
                        'image/gif'
                    ],
                    'messageType' => 'Avatar must be a png, gif, or jpg',
                    "maxResolution" => "600x600",
                    "messageMaxResolution" => "Avatar must not be bigger than 600x600 pixels",
                ]
            )
        );
    }
}

@kaioken
Copy link
Member

kaioken commented Jul 1, 2020

@ruudboon also encounter this issue

Here is the code

        $uploadConfig = [
            'maxSize' => '100M',
            'messageSize' => ':field exceeds the max filesize (:max)',
            'allowedTypes' => [
                'image/jpeg',
                'image/png',
                'image/webp',
                'audio/mpeg',
                'audio/mp3',
                'text/plain',
                'audio/mpeg',
                'application/pdf',
                'audio/mpeg3',
                'audio/x-mpeg-3',
                'application/x-zip-compressed',
                'application/octet-stream',
                'application/msword',
                'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
            ],
            'messageType' => 'Allowed file types are :types',
        ];

        $validator->add(
            'file',
            new FileValidator($uploadConfig)
        );

    
        //validate this form for password
        $validator->validate([
            'file' => [
                'name' => $file->getName(),
                'type' => $file->getType(),
                'tmp_name' => $file->getTempName(),
                'error' => $file->getError(),
                'size' => $file->getSize(),
            ]
        ]);

@ruudboon ruudboon removed the need script to reproduce Script is required to reproduce the issue label Jul 1, 2020
@ruudboon
Copy link
Member

ruudboon commented Jul 1, 2020

thnx @RayHughes @kaioken thnx Will have a look at it.

@kaioken
Copy link
Member

kaioken commented Jul 1, 2020

@ruudboon I believe this is due to the following

if !isset value["error"] || !isset value["tmp_name"] || value["error"] !== UPLOAD_ERR_OK || !is_uploaded_file(value["tmp_name"]) {

Phalcon is verifying this is a HTTP POST , this is an issue for me since I'm not, I'm just attaching the file to directly

Thanks

@ruudboon
Copy link
Member

ruudboon commented Jul 1, 2020

This validator is intended for checking php file uploads ($_FILES) so I think this behaviour is correct.
I do see the need for having a separate file validator that doesn't have anything to do with uploading files. Will check with the rest of the team if we can add this for 4.1

@ruudboon ruudboon added new feature request Planned Feature or New Feature Request and removed bug A bug report status: unverified Unverified labels Jul 1, 2020
@kaioken
Copy link
Member

kaioken commented Jul 1, 2020

@ruudboon yep that I get, but this behavior worked on 3.4 that why you are seeing use report it as a bug since we already had libraries expecting the result to work with $_FILES and by us passing a file

;) thanks

@Jeckerson Jeckerson added 5.0 The issues we want to solve in the 5.0 release and removed 4.1.0 labels Sep 30, 2020
@zsilbi zsilbi added the transfer label Oct 6, 2020
@niden niden linked a pull request Apr 22, 2021 that will close this issue
5 tasks
@niden
Copy link
Member

niden commented Apr 23, 2021

Resolved in #15432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release new feature request Planned Feature or New Feature Request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants