0

I have a php script where I store a file uploaded by the user in a tmp folder and then move it to s3. I also check if the emails they entered are valid in the database. I am using chunking so that large files can get uploaded to the server quickly. My post_max_size is 8M, however I am only able to upload files that are very small.

For example, If I upload a 7M file, I get an alert I set which says the file size is too large. This alert is only supposed to show if file size is >8M. If I upload a 1 KB file, it uploads the file successfully. Am I doing something incorrectly? I'm fairly new to php so I'd appreciate the help!

  • `post_max_size` and `upload_max_filesize` go hand in hand when handling file uploads. What is that value? – Zoli Szabó Jun 14 '21 at 14:39
  • Not your current issue but your you are only using `mysqli` safely once. The rest of your code is open to SQL injections. The code around `bind values from $tags to the query` is correct. The rest of `mysqli` usage is not safe. Parameterize everywhere – user3783243 Jun 14 '21 at 14:41
  • @ZoliSzabó post_max_size is 8M and upload_max_filesize is 2M. However, even if I upload a file that is 8 KB I get the alert. –  Jun 14 '21 at 14:44
  • @user3783243 will do! any solutions for the current issue I'm facing? –  Jun 14 '21 at 14:45
  • Where's the code that fires the `alert`? – user3783243 Jun 14 '21 at 15:01
  • @user3783243 the alert is fired from the else statement in the ajax success function. I just have simple code for an alert modal in my html file. But it shouldn't be going into that else statement in the first place since if ($_FILES['file']['size'] <= ini_get('post_max_size') should be true in the php check (line 3). –  Jun 14 '21 at 15:11

2 Answers2

0

ini_get('post_max_size') can (and will) return human-friendly values. So you will end up comparing integer file size values with string values (8M in your case). And 9000 > '8M'...

UPDATE: There is no need to explicitly use the PHP configuration values post_max_size or upload_max_filesize, because:

  1. in case of exceeding the post_max_size value, the $_FILES array will be empty, plus a warning will be generated, e.g.:

Warning: POST Content-Length of 3414 bytes exceeds the limit of 2048 bytes in Unknown on line 0

  1. in case of exceeding the upload_max_filesize value, the corresponding 'error' field(s) within the $_FILES array will contain the UPLOAD_ERR_INI_SIZE value, e.g:

array(1) { ["name"]=> array(5) { ["name"]=> string(12) "filename.ext" ["type"]=> string(0) "" ["tmp_name"]=> string(0) "" ["error"]=> int(1) ["size"]=> int(0) } }

All the above mean that if there's a non-empty $_FILES array available and the value of the error field is UPLOAD_ERR_OK, you can be sure that the upload was successful and configuration values were also respected.

In your case this translates to the condition if ($_FILES['file']['size'] <= ini_get('post_max_size')) ... not only incorrect (comparing integer file sizes (e.g. 9000) with possibly non-integer strings (e.g. '8M')), but also not needed at all.

Zoli Szabó
  • 4,366
  • 1
  • 13
  • 19
  • I changed it to '8M' but I am still getting the alert for files smaller than 8M. –  Jun 14 '21 at 15:23
  • Let me repeat what I wrote again: `ini_get('post_max_size')` will return in your case `'8M'` as-is. Not `8388608`. So you cannot compare integer values coming from `$_FILES['file']['size']` with the value from `ini_get('post_max_size')`, unless you previously convert the human-friendly value `'8M'` to the machine-friendly `8388608`. Otherwise the comparison is not correct. – Zoli Szabó Jun 14 '21 at 15:29
  • Ohh I see what you mean. Should I use $_SERVER['CONTENT_LENGTH'] to make the comparison with ini_get('post_max_size') then? –  Jun 14 '21 at 15:38
  • Ok so if I remove that if statement altogether, I should be good right? Because my first two if statements are basically checking that $_FILES array is not empty and upload error is ok. –  Jun 14 '21 at 16:55
  • Yes, `array_key_exists('file', $_FILES) && ($_FILES['file']['error'] === UPLOAD_ERR_OK)` is enough to make sure that file sizes are alright (and also to eliminate other upload error cases). – Zoli Szabó Jun 14 '21 at 17:02
  • It's still not accepting all files under 8M. I made the changes you suggested –  Jun 14 '21 at 18:33
  • Which condition is failing? – Zoli Szabó Jun 15 '21 at 12:58
  • `if ($_FILES['file']['error'] === UPLOAD_ERR_OK)` is the condition that failed. –  Jun 15 '21 at 21:17
  • What value does `$_FILES['file']['error']` contain? – Zoli Szabó Jun 15 '21 at 21:23
  • The value is 1 when that condition fails –  Jun 16 '21 at 15:34
  • Value `1` is `UPLOAD_ERR_INI_SIZE` meaning "The uploaded file exceeds the upload_max_filesize directive in php.ini". Are you 100% sure the `upload_max_filesize` config value is larger than the size of the file you want to upload? – Zoli Szabó Jun 16 '21 at 16:56
  • Nevermind, I was being stupid lol. It's working, thanks for the help! –  Jun 17 '21 at 15:37
0

I'm not sure how relevant this may be, but I noticed that you haven't specified anywhere that the type of the data is "multipart/form-data". I've always been under the impression that a file upload won't work correctly unless this is specified.

Also not related per se but I see you're using jQuery. I recommend moving away from that and using the native fetch API. It's a cleaner and more modern solution.

Michael Cook
  • 86
  • 1
  • 6
  • I am using multipart/form-data in my HTML form code. I haven't included that code because it's not super relevant to the issue I'm having right now. –  Jun 14 '21 at 15:25