File upload bugs

[ghost]

Few bugs(?) I noticed when uploading a file.

So here's my controller method code for uploading user avatar:

$file = $this->request->getFile('avatar');

if ($file->isValid() && ! $file->hasMoved()) {

    $fileName = $file->getRandomName();
    $file->move(WRITEPATH.'uploads', $fileName);

}

First error I get is this:

unlink(/private/var/tmp/php2jFbtU): No such file or directory

The file is actually uploaded and is in writable/uploads directory, meanwhile the upload class chmods my writable/uploads directory to 0666 and I no longer can access uploaded images.

So second time I try to upload the image, I get this error:

move_uploaded_file([codeigniter4]/writable/uploads/1471438186_b6d0d7b87430fb5e6b7c.jpg): failed to open stream: Permission denied

So I chmod writable/uploads back to 0777 and my fix is following:

I commented these two lines in system/HTTP/Files/UploadedFile.php:182 :

// @chmod($targetPath, 0666 & ~umask());

// unlink($this->path);

Now obviously I don't get these errors anymore about unlink and permissions, but then there's a further error:

Return value of CodeIgniter\HTTP\Files\UploadedFile::move() must be of the type boolean, none returned

From error, I see that it should return a boolean, but there's nothing being returned from the move function, and what exactly should it return?

If it shouldn't return anything then there's just easy pull request to fix code with return hinting.

The issue is - I don't exactly know the desired behaviour of file upload, so I cannot create pull request with bugfixes, or maybe my controller method code isn't right.

[lonnieezell]

You're right, there's a few issues here:

1 - unlinking on line 184 is redundant since, if the file has successfully moved, it won't be there. That can go away.
2 - I was using the wrong value for chmod to begin with, which was ending up with unreadable folders after applying the umask.
3 - There's no reason that the function really needed to return a boolean.

I've made the modifications, and am pushing up a fix.

Thanks!