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

Bug: Codeigniter doesn't recognize file upload data for large files #8403

Open
dusanbrankov opened this issue Jan 5, 2024 · 9 comments
Open
Labels
waiting for info Issues or pull requests that need further clarification from the author

Comments

@dusanbrankov
Copy link

dusanbrankov commented Jan 5, 2024

PHP Version

8.1

CodeIgniter4 Version

4.4.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

MySQL version 1.24.3

What happened?

According to the CodeIgniter 4.4.4 documentation on Working with uploaded files, the isValid() method should return an error, when trying to upload a file that exceeds the upload_max_filesize defined in php.ini, which in my case is set to 2MB.

When I try to upload a file larger than 2MB, it doesn't catch the error, more importantly, it doesn't recognize the name value of the given file input, so it seems that I can't check the file size manually either.

For example, given the following code:

<input type="file" id="image-upload" name="images[]" multiple>
$files = $this->request->getFiles("images");

foreach ($files["images"] as $file) {
    if (! $file->isValid()) {
        $error_code = $file->getError();
        throw new RuntimeException(
            $file->getErrorString()." ".$error_code
        );
    }

    // ...
}

If the file is too large, I get the following error:

Undefined array key "images" 

I've also tried using the validate() method as described in the documentation, but I get the same error.

Looking for a solution, I came across this forum post where it is suggested to change the upload_max_filesize in php.ini. But I don't want to do that because not every hosting provider gives you the control to change that. However, for testing purposes, I changed it, restarted my Apache server and tried again, but I still get the same error.

When I try the same thing in plain PHP, it works as expected because I can access the file data and thus check for the file size.

Steps to Reproduce

See "What happened?" section.

Expected Output

I should get an UPLOAD_ERR_INI_SIZE error message.

Anything else?

Note that I've tested it on my local machine with Apache 2.0.

@dusanbrankov dusanbrankov added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 5, 2024
@michalsn
Copy link
Member

michalsn commented Jan 5, 2024

I cannot reproduce the problem.

Can you provide the full logic of your example controller method? Also the $this->request->getFiles() doesn't accept any parameters. What exact version of the framework are you using? v4.4.4?

The only scenario when it will not work is when the post_max_size is the same as upload_max_filesize.

@dusanbrankov
Copy link
Author

Hi @michalsn, thanks for the quick reply. You're right, I removed the redundant parameter from getFiles(), don't know how it got there.

I've just tried again to upload a 9MB image with the following settings in php.ini:

upload_max_filesize = 20M

; Value may be 0 to disable the limit (ok, let's try that)
post_max_size = 0

But unfortunately it still doesn't work. Also, I just noticed that I also can't access POST data when trying to upload large files. So not only the FILES data is affected. Really weird!

I'm using CodeIgniter version 4.4.3. It's a new project, created about two weeks ago.

Can you provide the full logic of your example controller method?

Sure thing, here's a simplified version of my logic to create a new entry, I just removed some error checks and other unimportant stuff:

public function create()
{
    $product = new Product;
    $formdata = $this->getFormData();

    $product->fill($formdata["input"]);

    $id = $this->model->insert($product);

    if ($formdata["files_exist"]) {
        $filestack = $this->upload($id, $formdata["files"]);

        if ($filestack === false) {
            return redirect()
                ->back()
                ->with("errors", ["File too large"]);
        }

        $product->image = implode(", ", $filestack);
        $this->model->update($id, $product);
    }

    return redirect()
        ->to("products/$id")
        ->with("message", "Product saved.");
}

private function getFormData(?Product $product = null): array
{
    $files = $this->request->getFiles();
    $post_data = $this->request->getPost();
    
    // for testing purposes only
    dd($files);

    $files_exist = isset($files["images"])
        ? $files["images"][0]->getSize() > 0
        : false;

    // insert "placeholder" to avoid "nothing to update" error
    $filestack = ($product && $product->image)
        ? $product->image . ", placeholder"
        : "placeholder";

    $file_data = ["images" => $filestack];

    $input = $files_exist
        ? array_merge($post_data, $file_data)
        : $post_data;

    return [
        "files" => $files,
        "files_exist" => $files_exist,
        "input" => $input,
    ];
}

private function upload(string $id, array $files): array | bool
{
    $filestack = [];
    $file_too_large = false;

    foreach ($files["images"] as $file) {
        if (! $file->isValid()) {
            $error_code = $file->getError();
            if ($error_code !== UPLOAD_ERR_NO_FILE) {
                throw new RuntimeException(
                    $file->getErrorString()." ".$error_code
                );
            }
        }

        // for testing purposes only
        if ($file->getSizeByUnit("mb") > 2) {
            $file_too_large = true;
            break;
        }

        $filename = $file->getRandomName();

        $file->move($this->upload_dir . "/products/$id/", $filename);

        array_push($filestack, $filename);

    }

    return $file_too_large ? false : $filestack;
}

Let me know if you need more information.

@michalsn
Copy link
Member

michalsn commented Jan 5, 2024

I think the problem is your implementation. When the file size exceeds the allowed size set in php.ini, you will not get the expected value from methods like: getSize() or getSizeByUnit() they return 0.

@michalsn michalsn removed the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 5, 2024
@dusanbrankov
Copy link
Author

dusanbrankov commented Jan 5, 2024

It has nothing to do with that, I just added getSizeByUnit() to test if it does something, that's why I added the comments. Also, the isValid() method is called first, so it should throw a RuntimeException. According to the documentation it should throw an error if:

The file exceeds your upload_max_filesize ini directive.

I shared my updated php.ini settings in my previous comment. Again, I set it to 20M and tried to upload a 9MB image.

You should also notice, that dd($files) runs pretty early, 95% of the shared code doesn't get executed. As I said, I can't access any form data ($_FILES and $_POST) when trying to upload large files, as if no POST request was sent.

Also, may I ask why you removed the "bug" label?

@michalsn
Copy link
Member

michalsn commented Jan 6, 2024

I focused on the original error that was reported, which was the absence of an error when upload_max_filesize is exceeded. For now, I will be referring only to this issue.

It has nothing to do with that, I just added getSizeByUnit() to test if it does something, that's why I added the comments. Also, the isValid() method is called first, so it should throw a RuntimeException.

Please notice what values are returned when you check the results of dd($files), the property size will be 0. Therefore all the logic behind the $files_exist variable will not be executed (because $files["images"][0]->getSize() > 0 will be false).

Also, may I ask why you removed the "bug" label?

The "bug" label was removed because at this point I don't think this is a bug in the framework. As I mentioned earlier, I checked the code from your original report and it's working just fine.


As I said, I can't access any form data ($_FILES and $_POST) when trying to upload large files, as if no POST request was sent.

There is nothing that would prevent that. Please make sure and call dd($formdata) right after $formdata = $this->getFormData().

Overall, I would suggest to post this on the forum. You may find more help there since we won't be debugging your code here unless you provide a clear example for a bug.

@dusanbrankov
Copy link
Author

I've commented out all the code to show you what I mean:

public function create()
{
    dd($this->request->getPost());
}

Now, when I fill out my form, I get the POST data as expected. However, when I do the same, but with the 9MB image I mentioned earlier, I get:

$this->request->getPost() array (0)

It's the same with getFiles(), I get an empty array unless I upload a file under 2MB.

In php.ini I still have the following settings:

upload_max_filesize = 20M
post_max_size = 0

I am not asking for help to debug my code, I just find it strange that I cannot access any $_POST or $_FILES data when trying to upload a file that is within the upload_max_filesize range. I think it's pointless to test this further with the isValid() method as I initially did because of the missing form data.

I also tested it with plain PHP and it works as expected; I always get the complete form data regardless of file size.

@michalsn
Copy link
Member

michalsn commented Jan 6, 2024

I also tested it with plain PHP and it works as expected; I always get the complete form data regardless of file size.

Are you doing this in the same environment? What do you get when you call:

dd(ini_get('upload_max_filesize'), ini_get('post_max_size'), ini_get('memory_limit'));

@dusanbrankov
Copy link
Author

Yes, it's the same environment.

ini_get(...) string (3) "20M"
ini_get(...) string (1) "0"
ini_get(...) string (4) "128M"

I will now restore the settings as they were originally, as it doesn't make any difference in this particular case. I will see if I can get it to work on my secondary machine (same OS and server) when I have more time.

@michalsn
Copy link
Member

michalsn commented Jan 6, 2024

The only thing I have found is this: https://www.php.net/manual/en/ini.core.php#ini.post-max-size

If the size of post data is greater than post_max_size, the $_POST and $_FILES superglobals are empty.

If you gonna test it more, please make some really simple methods that will only display data that has been sent, like:

public function create()
{
    dd($this->request->getFiles(), $this->request->getPost());
}

And without using our methods (to see if there are any differences):

public function create()
{
    dd($_FILES, $_POST);
}

If there will be no differences, please try also plain PHP (without framework):

var_dump($_FILES, $_POST);

Please, make your code the bare minimum when you write the test code. Also, please post settings for upload_max_filesize, post_max_size, and memory_limit during these tests, and let's stick to them (at this point I'm lost about what was the default one). Thanks.

@michalsn michalsn added the waiting for info Issues or pull requests that need further clarification from the author label Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for info Issues or pull requests that need further clarification from the author
Projects
None yet
Development

No branches or pull requests

2 participants