Hello Internals!
I originally posted this to Github as an issue -
https://github.com/php/php-src/issues/20456 - but someone there
recommended that I post it to this list instead. Including the same
content here.
If you run this script:
<html><title>test</title> <body> <form method="POST" action=""> <?php for($i=0;$i<1200;$i++) { print "<input type='hidden' name='test[]' value='$i' />"; } ?> <input type="submit"> </form> <?php if(isset($_REQUEST['test'])) { print "<div>Total test values: ".count($_REQUEST['test'])."</div>"; } ?> </body> </html>You end up with this:
Warning: PHP Request Startup: Input variables exceeded 1000. To
increase the limit change max_input_vars in php.ini. in Unknown on
line 0
Total test values: 1001
Instead of the expected:
Total test values: 1200
That warning is nice and all, but in the world of modern PHP with a
framework like Laravel - too much has changed or is changing, such
that a default installation of at Laravel suppresses all warnings,
because they've grown too noisy. My boss and I lost two days
troubleshooting this, and we ended up actually mangling user data in
the process.
I'm wondering if there isn't a better way to handle this. I can think
of a few options -
- Get rid of the option entirely (ignoring it in php.ini). Just let
people submit as much stuff as they want. I don't know enough about
the technical implementation to say whether or not that's a good idea.
1.a Get rid of the option for 64-bit machines. If the hash table
collision problem that is mentioned in the documentation for this
setting is only ameliorated on 64-bit machines, then this might do the
trick. - Return a 400-series code - perhaps "413 Payload Too Large". This
would've allowed us to at least find the problem much sooner. - Return a 500-series code - I like this less, but we still would've
at least found the issue if this had happened. And you could argue
that it's the server's fault that it can't handle all of those
variables. - If the problem is something to do with hash tables, maybe we can
relax the variable count to only include variables with different
names - permitting our very-large array to be submitted correctly? - Return an error instead of a warning - that would've at least shown up.
- Something else?
What I think is not good is just (semi-)silently truncating the
input that's being submitted from the form. For our use-case, that
resulted in some users actually losing data - removing group
memberships for whichever of their users was at the 'end' and making
it hard to change the actual permissions of the group (which show up
'after' the user list, so they were getting chopped off the end).
We've worked around the problem by smashing all the values together
using Javascript and submitting them as one big giant comma-delimited
list, but I would much rather us not having to do that - nor do I want
other developers to run into this problem and have to do similar
workarounds.
My C is pretty rusty, but I'm happy to try and take a swing at
changing this if we can come up with a decision on which approach we
think is best, if any.
Thank you so much, always, for making PHP! It's been my daily-driver
programming language for probably around 25 years now :)
-B.
Hi Brady,
I agree that E_WARNING is a poor way to handle this limit, and IMO a fatal
error should be triggered instead. The ability to suppress and ignore is
the root cause of why your situation is possible at all, and Laravel's
behavior in this instance also did you a massive disservice.
That being said however, this is also an extreme and self-inflicted edge
case. 1k is an absurd number, even 100 input vars should be a sign of poor
code logic. I urge you to redesign your solution entirely instead of
looking for a quick workaround.
Cheers,
Andrey.
Hi Brady,
I agree that
E_WARNINGis a poor way to handle this limit, and IMO a fatal
error should be triggered instead. The ability to suppress and ignore is
the root cause of why your situation is possible at all, and Laravel's
behavior in this instance also did you a massive disservice.That being said however, this is also an extreme and self-inflicted edge
case. 1k is an absurd number, even 100 input vars should be a sign of poor
code logic. I urge you to redesign your solution entirely instead of
looking for a quick workaround.Cheers,
Andrey.
Unfortunately I'm no stranger to max input vars. We have increased the
limit to 10k because we will frequently hit over 1k. PHP is used for more
than just websites. One example is having a range of 20+ shoe sizes with
100+ stores in a single form where you can enter a number per size per
store. These forms are not rare in the application my company develops and
there's not really another way to deal with this. There's no performance
issue here and it works just fine, other than being bitten by an invisible
issue that causes data loss.
Having a fatal error would certainly help a lot to at least prevent partial
data from being processed and potentially causing data corruption.
Hi Brady,
I agree that
E_WARNINGis a poor way to handle this limit, and IMO a
fatal error should be triggered instead. The ability to suppress and ignore
is the root cause of why your situation is possible at all, and Laravel's
behavior in this instance also did you a massive disservice.That being said however, this is also an extreme and self-inflicted edge
case. 1k is an absurd number, even 100 input vars should be a sign of poor
code logic. I urge you to redesign your solution entirely instead of
looking for a quick workaround.Cheers,
Andrey.Unfortunately I'm no stranger to max input vars. We have increased the
limit to 10k because we will frequently hit over 1k. PHP is used for more
than just websites. One example is having a range of 20+ shoe sizes with
100+ stores in a single form where you can enter a number per size per
store. These forms are not rare in the application my company develops and
there's not really another way to deal with this. There's no performance
issue here and it works just fine, other than being bitten by an invisible
issue that causes data loss.Having a fatal error would certainly help a lot to at least prevent
partial data from being processed and potentially causing data corruption.
Honestly I really do not understand why you call that an " invisible issue".
It is emitting a warning all the time, it is your job as a professional
developer to catch all warnings at least in the development phase.
Hi Brady,
I agree that
E_WARNINGis a poor way to handle this limit, and IMO a
fatal error should be triggered instead. The ability to suppress and ignore
is the root cause of why your situation is possible at all, and Laravel's
behavior in this instance also did you a massive disservice.That being said however, this is also an extreme and self-inflicted edge
case. 1k is an absurd number, even 100 input vars should be a sign of poor
code logic. I urge you to redesign your solution entirely instead of
looking for a quick workaround.Cheers,
Andrey.Unfortunately I'm no stranger to max input vars. We have increased the
limit to 10k because we will frequently hit over 1k. PHP is used for more
than just websites. One example is having a range of 20+ shoe sizes with
100+ stores in a single form where you can enter a number per size per
store. These forms are not rare in the application my company develops and
there's not really another way to deal with this. There's no performance
issue here and it works just fine, other than being bitten by an invisible
issue that causes data loss.Having a fatal error would certainly help a lot to at least prevent
partial data from being processed and potentially causing data corruption.Honestly I really do not understand why you call that an " invisible
issue".
It is emitting a warning all the time, it is your job as a professional
developer to catch all warnings at least in the development phase.
I can't think of every single possible combination in development, this is
all based on tenants and their setup. That said, a ton of this is also code
that was written way before I started working here (20 years+), and we're
talking a million+ lines of code. This warning disappears between millions
of log lines in production. I would rather have a customer call us with an
error than the issue silently dodging detection. I'm not looking for your
approval or anything, just explaining that this can pose a real issue no
matter how hard you try to do it the right way.
To give you a little more context - I have to support 3 or 4 different
versions of PHP, with various differing levels of features being
mid-deprecation or emitting warnings. I also have 260 composer
dependencies that I am not in direct control of, which might or might
not emit their own warnings/errors/etc. If I had warnings emitting, I
wouldn't have ever caught it over the noise of everything else that's
constantly making noise.
I absolutely did used to develop with warnings on, and then run in
production without them on, when I was working on software that had
one single deployment, and running on one specific version of PHP.
But, unfortunately, that's not my situation anymore. (Our software is
open-source, and it needs to run on the widest range of PHP versions
we can possibly support).
And even if we did run with those warnings enabled, I don't think I
would've caught it. Because you need to have over ~1000 users in your
database to trigger the issue, and I don't have that on my development
workstation. We, alas, discovered the issue in production, on real
customer data - and the production environment does not emit
deprecation notices nor warnings nor the like, as you might expect.
-B.
Hi Brady,
I agree that
E_WARNINGis a poor way to handle this limit, and IMO a fatal error should be triggered instead. The ability to suppress and ignore is the root cause of why your situation is possible at all, and Laravel's behavior in this instance also did you a massive disservice.That being said however, this is also an extreme and self-inflicted edge case. 1k is an absurd number, even 100 input vars should be a sign of poor code logic. I urge you to redesign your solution entirely instead of looking for a quick workaround.
Cheers,
Andrey.Unfortunately I'm no stranger to max input vars. We have increased the limit to 10k because we will frequently hit over 1k. PHP is used for more than just websites. One example is having a range of 20+ shoe sizes with 100+ stores in a single form where you can enter a number per size per store. These forms are not rare in the application my company develops and there's not really another way to deal with this. There's no performance issue here and it works just fine, other than being bitten by an invisible issue that causes data loss.
Having a fatal error would certainly help a lot to at least prevent partial data from being processed and potentially causing data corruption.
Honestly I really do not understand why you call that an " invisible issue".
It is emitting a warning all the time, it is your job as a professional developer to catch all warnings at least in the development phase.
Hi Brady,
I agree that
E_WARNINGis a poor way to handle this limit, and IMO a fatal error should be triggered instead. The ability to suppress and ignore is the root cause of why your situation is possible at all, and Laravel's behavior in this instance also did you a massive disservice.That being said however, this is also an extreme and self-inflicted edge case. 1k is an absurd number, even 100 input vars should be a sign of poor code logic. I urge you to redesign your solution entirely instead of looking for a quick workaround.
In our case, we had a multi-select (<select> with the 'multiple'
attribute) that listed a few thousand users. You can select them all,
and each selected value counts towards the 1000 fields being
submitted. You could argue that it's poor design, and, maybe, it could
be. But it's still a valid use-case IMHO.
-B.