Front end file upload giving confusing error message when no file selected

I’ve been using the code from the Uploading files from frontend cookbook for a site I’m building.

I’ve noticed when I click submit without having selected a file the error message given back is, for example:

The extensions for “28069609” is missing

As I understand it the controller gives a unique number to prefix the filename to hinder opening of malicious uploads. But this is messing with the error check as it’s creating a filename for a file that isn’t there, without an extension.

It would make more sense to the user if the error message given was “No file selected”. How can I most easily fix this? Do I need a check in place before the file name appending happens?

Many thanks!

I’d do it after this bit

    if (count($uploads) > 3) {
      $alerts['exceedMax'] = 'You may only upload 3 files.';
      return compact('alerts', 'success');
    }

Here we check if the number of uploads exceeds 3, so you could do the same for no uploads, wrapping the rest of the code inside that if statement

So would it look something like this? I’m still pretty new to php so forgive me if this is dumb :stuck_out_tongue:

    if (count($uploads) > 3) {
  $alerts['exceedMax'] = 'You may only upload 3 files.';
  return compact('alerts', 'success');
}

if (count($uploads) == 0) {
  $alerts['noFile'] = 'You have not selected a file.';
  return compact('alerts', 'success');
}

This is still returning the same error so I’m guessing I’ve done it incorrectly.

I forgot that you get an array anyway, so we have to change the code:

<?php

return function ($kirby, $page) {

  $alerts  = [];
  $success = '';

  if ($kirby->request()->is('post') === true && get('submit')) {

    // check the honeypot
    if (empty(get('website')) === false) {
      go($page->url());
      exit();
    }

    $uploads = $kirby->request()->files()->get('file');

    // we only want 3 files
    if (count($uploads) > 3) {
      $alerts['exceedMax'] = 'You may only upload 3 files.';
      return compact('alerts', 'success');
    }

    // authenticate as almighty
    $kirby->impersonate('kirby');

    foreach ($uploads as $upload) {
      if ($upload['error'] !== 0) {

      
        // check for duplicate
        $files      = page('storage')->files();
        $duplicates = $files->filter(function ($file) use ($upload) {
            // get original safename without prefix
            $pos              = strpos($file->filename(), '_');
            $originalSafename = substr($file->filename(), $pos + 1);

            return $originalSafename === F::safeName($upload['name']) &&
                    $file->mime() === $upload['type'] &&
                    $file->size() === $upload['size'];
        });

        if ($duplicates->count() > 0) {
            $alerts[$upload['name']] = "The file already exists";
            continue;
        }

        try {
            $name = crc32($upload['name'].microtime()). '_' . $upload['name'];
            $file = page('storage')->createFile([
            'source'   => $upload['tmp_name'],
            'filename' => $name,
            'template' => 'upload',
            'content' => [
                'date' => date('Y-m-d h:m')
            ]
            ]);
            $success = 'Your file upload was successful';
        } catch (Exception $e) {
            $alerts[$upload['name']] = $e->getMessage();
        }
      } else {
          // throw error depending on error code, error code 4 = no file
      }
    }
  }

  return compact('alerts', 'success');
};

Hm I tried this but I’m still getting the same error :confused:

Is there supposed to be some more code in the final “else” bracket? Where it says

// throw error depending on error code, error code 4 = no file

?

Yes, there is supposed to be some more code, if error code is 4, throw error no file etc.

Ok I’ll try and work it out, thanks!