Error when POST / upload limits are exceeded

  107354
October 1, 2019 08:00 benjamin.morel@gmail.com (Benjamin Morel)
Hi internals,

One thing that always bugged me is how PHP silently ignores data that
exceeds the limits defined in php.ini.

For example, posting a form exceeding post_max_size results in empty $_POST
and $_FILES, with no error message whatsoever. This has bugged a few more
people than just myself, if I believe this StackOverflow question
<https://stackoverflow.com/q/2133652/759866> with 37k views.

Another example is max_file_uploads: if I upload more files that the
configured value, the subsequent files are silently ignored, and only the
first n files appear in $_FILES.

I'd like to work on an RFC to make all these errors in PHP 8 (ideally with
a way to catch the error, but I'm not sure how to do that), but would love
to get some feedback/ideas before venturing in an RFC.

Thank you.

— Benjamin
  107355
October 1, 2019 12:22 rasmus@lerdorf.com (Rasmus Lerdorf)
On Tue, Oct 1, 2019 at 1:00 AM Benjamin Morel morel@gmail.com>
wrote:

> Hi internals, > > One thing that always bugged me is how PHP silently ignores data that > exceeds the limits defined in php.ini. > > For example, posting a form exceeding post_max_size results in empty $_POST > and $_FILES, with no error message whatsoever. This has bugged a few more > people than just myself, if I believe this StackOverflow question > <https://stackoverflow.com/q/2133652/759866> with 37k views. > > Another example is max_file_uploads: if I upload more files that the > configured value, the subsequent files are silently ignored, and only the > first n files appear in $_FILES. > > I'd like to work on an RFC to make all these errors in PHP 8 (ideally with > a way to catch the error, but I'm not sure how to do that), but would love > to get some feedback/ideas before venturing in an RFC. >
We have https://www.php.net/manual/en/features.file-upload.errors.php but in general this is not so easy to make more convenient because this runs before your PHP code starts to run, so it is not like you can stick it in a try/catch. -Rasmus
  107356
October 1, 2019 13:32 benjamin.morel@gmail.com (Benjamin Morel)
> We have https://www.php.net/manual/en/features.file-upload.errors.php but in general this is not so easy to make more convenient because this runs
before your PHP code starts to run, so it is not like you can stick it in a try/catch. Hi Rasmus, These errors are only relevant when you have something in $_FILES, which is not always the case I'm afraid: - when exceeding post_max_size, $_FILES is empty - when exceeding max_file_uploads, you don't get entries in $_FILES past the nth allowed file I understand that because all of this happens before userland code is run, indeed it's not possible to catch it. I also understand that throwing a fatal error in this case would not be welcome, as it does not give userland code a chance to inform the user about the problem. What about setting a $_SERVER value in this case? For example: $_SERVER['CONFIG_LIMIT_EXCEEDED'] = 'post_max_size' This, together with empty $_POST and $_FILES if any limit is exceeded (including max_file_uploads), should be relatively sane behaviour IMO. — Benjamin
  107357
October 1, 2019 15:16 rasmus@lerdorf.com (Rasmus Lerdorf)
On Tue, Oct 1, 2019 at 6:32 AM Benjamin Morel morel@gmail.com>
wrote:

> > We have https://www.php.net/manual/en/features.file-upload.errors.php but > in general this is not so easy to make more convenient because this runs > before your PHP code starts to run, so it is not like you can stick it in a > try/catch. > > Hi Rasmus, > > These errors are only relevant when you have something in $_FILES, which > is not always the case I'm afraid: > > - when exceeding post_max_size, $_FILES is empty > - when exceeding max_file_uploads, you don't get entries in $_FILES past > the nth allowed file > > I understand that because all of this happens before userland code is run, > indeed it's not possible to catch it. > I also understand that throwing a fatal error in this case would not be > welcome, as it does not give userland code a chance to inform the user > about the problem. > > What about setting a $_SERVER value in this case? For example: > > $_SERVER['CONFIG_LIMIT_EXCEEDED'] = 'post_max_size' > > This, together with empty $_POST and $_FILES if any limit is exceeded > (including max_file_uploads), should be relatively sane behaviour IMO. >
Perhaps a more generic $_SERVER['PHP_REQUEST_STATUS'] or something along those lines where you'd put the error message from https://www.php.net/manual/en/features.file-upload.errors.php as well. And add new states for these exceeded limits that aren't caught there. It would be nice if you just needed a single check instead of having to look for multiple things. -Rasmus
  107358
October 1, 2019 15:25 benjamin.morel@gmail.com (Benjamin Morel)
> Perhaps a more generic $_SERVER['PHP_REQUEST_STATUS'] or something along those lines where you'd put the error message from
https://www.php.net/manual/en/features.file-upload.errors.php as well. And add new states for these exceeded limits that aren't caught there. It would be nice if you just needed a single check instead of having to look for multiple things. Those are per-file error codes, that belong in each $_FILES entry, while the errors I'm talking about affect the whole request, so I'm afraid you cannot put these errors in the same place, nor can you extend the existing error codes, as they do not have the same scope! — Benjamin
  107359
October 1, 2019 20:26 rasmus@lerdorf.com (Rasmus Lerdorf)
On Tue, Oct 1, 2019 at 8:25 AM Benjamin Morel morel@gmail.com>
wrote:

> > Perhaps a more generic $_SERVER['PHP_REQUEST_STATUS'] or something along > those lines where you'd put the error message from > https://www.php.net/manual/en/features.file-upload.errors.php as well. > And add new states for these exceeded limits that aren't caught there. It > would be nice if you just needed a single check instead of having to look > for multiple things. > > Those are per-file error codes, that belong in each $_FILES entry, while > the errors I'm talking about affect the whole request, so I'm afraid you > cannot put these errors in the same place, nor can you extend the existing > error codes, as they do not have the same scope! >
I know they are per-file errors. I wrote that code :) But you could still have a global status that specifies whether an error occurred or not. Then you can go look for which file it occurred on in the $_FILES array. The idea is to have a single check that tells you if everything was fine on the request. -Rasmus
  107360
October 1, 2019 21:39 thruska@cubiclesoft.com (Thomas Hruska)
On 10/1/2019 1:26 PM, Rasmus Lerdorf wrote:
> On Tue, Oct 1, 2019 at 8:25 AM Benjamin Morel morel@gmail.com> > wrote: > >>> Perhaps a more generic $_SERVER['PHP_REQUEST_STATUS'] or something along >> those lines where you'd put the error message from >> https://www.php.net/manual/en/features.file-upload.errors.php as well. >> And add new states for these exceeded limits that aren't caught there. It >> would be nice if you just needed a single check instead of having to look >> for multiple things. >> >> Those are per-file error codes, that belong in each $_FILES entry, while >> the errors I'm talking about affect the whole request, so I'm afraid you >> cannot put these errors in the same place, nor can you extend the existing >> error codes, as they do not have the same scope! >> > > I know they are per-file errors. I wrote that code :) > > But you could still have a global status that specifies whether an error > occurred or not. Then you can go look for which file it occurred on in the > $_FILES array. The idea is to have a single check that tells you if > everything was fine on the request. > > -Rasmus
I agree this is needed. The problem I've encountered is that if there are any more variables after a limit is hit, then some of the submitted vars don't exist in the superglobals. Specifically, the variables before the limit do exist while the ones after don't. There's a LOT of userland code that *assumes* all expected non-file POST vars exist and then DO things with the non-existent variables in this scenario. Is it poorly written, lazy userland code? Sure, but it's certainly unexpected. In the past, I've seen all kinds of weird behaviors happen due to missing vars in userland ranging from looping, WSODs, and altering PHP sessions. The data gets submitted to the server but the PHP input parser ignores it once a limit is reached. One of the first things I do on a new system is to bump up the default limits considerably to avoid hitting the limits. The ability to globally detect that processing of input variables was early-terminated by the parser would allow userland startup sequences to detect the problem globally and cleanly terminate the request BEFORE the core application logic is encountered. This shouldn't just be for files, it should be for any time the parser early-terminates input processing but then proceeds to start executing userland code as if nothing is wrong. -- Thomas Hruska CubicleSoft President I've got great, time saving software that you will find useful. http://cubiclesoft.com/ And once you find my software useful: http://cubiclesoft.com/donate/
  107369
October 4, 2019 11:01 bishop@php.net (Bishop Bettini)
On Tue, Oct 1, 2019 at 5:39 PM Thomas Hruska <thruska@cubiclesoft.com>
wrote:

> On 10/1/2019 1:26 PM, Rasmus Lerdorf wrote: > > On Tue, Oct 1, 2019 at 8:25 AM Benjamin Morel morel@gmail.com> > > wrote: > > > >>> Perhaps a more generic $_SERVER['PHP_REQUEST_STATUS'] or something > along > >> those lines where you'd put the error message from > >> https://www.php.net/manual/en/features.file-upload.errors.php as well. > >> And add new states for these exceeded limits that aren't caught there. > It > >> would be nice if you just needed a single check instead of having to > look > >> for multiple things. > >> > >> Those are per-file error codes, that belong in each $_FILES entry, while > >> the errors I'm talking about affect the whole request, so I'm afraid you > >> cannot put these errors in the same place, nor can you extend the > existing > >> error codes, as they do not have the same scope! > >> > > > > I know they are per-file errors. I wrote that code :) > > > > But you could still have a global status that specifies whether an error > > occurred or not. Then you can go look for which file it occurred on in > the > > $_FILES array. The idea is to have a single check that tells you if > > everything was fine on the request. > > I agree this is needed. The problem I've encountered is that if there > are any more variables after a limit is hit, then some of the submitted > vars don't exist in the superglobals. Specifically, the variables > before the limit do exist while the ones after don't. There's a LOT of > userland code that *assumes* all expected non-file POST vars exist and > then DO things with the non-existent variables in this scenario. Is it > poorly written, lazy userland code? Sure, but it's certainly unexpected. > > In the past, I've seen all kinds of weird behaviors happen due to > missing vars in userland ranging from looping, WSODs, and altering PHP > sessions. The data gets submitted to the server but the PHP input > parser ignores it once a limit is reached. One of the first things I do > on a new system is to bump up the default limits considerably to avoid > hitting the limits. > > The ability to globally detect that processing of input variables was > early-terminated by the parser would allow userland startup sequences to > detect the problem globally and cleanly terminate the request BEFORE the > core application logic is encountered. This shouldn't just be for > files, it should be for any time the parser early-terminates input > processing but then proceeds to start executing userland code as if > nothing is wrong. >
It's possible, in a limited fashion, to detect and react to some startup errors: $ cat index.php http://127.0.0.1:8000/?a=1&b=2' array(4) { ["type"]=> int(2) ["message"]=> string(92) "Unknown: Input variables exceeded 1. To increase the limit change max_input_vars in php.ini." ["file"]=> string(7) "Unknown" ["line"]=> int(0) } Obviously this only gets the last one, and it doesn't handle situations like "the file pointed to by auto_prepend_file doesn't exist", but it seems a reasonable first step to assert error_get_last is null on script invocation. That said, I would prefer a function to a server/environment variable: error_get_startup() or something similar.