Robert Sossomon wrote:
> foreach ( $_POST as $key => $value )
> {
> while ($key != "submit")
> {
> \$$key = $value;
> $addtocart .= ",'\$$key'";
> }
> }
>
> The problem is that it seems to be hanging at the while loop. Is there a
> cleaner way to write the foreach loop so that it will dump out the
> "submit" key?
> I've been trying different combinations, and am sure it is something
> simple I
> have boffed.
Think about what happens with the first key/value pair.
Let's assume it's $_POST['name'] = "Robert";
Here's what you have, with the simple substitution of 'name' for $key and
"Robert" for $value:
while ('name' != 'submit')
{
$name = 'Robert';
$addtocart .= ",'\$name'";
}
Now, inside the while loop, when do you expect $key to change from 'name'
to 'submit'?...
Never gonna happen.
You're not changing $key at all in that inside loop.
You've sent PHP into a tail-spin of an infinite loop where it does the
same thing over and over and over ...
$name = 'Robert';
$addtocart .= ",'\$name'";
Actually, it tacks on '$name' to the $addtocart a zillion times, so it's
not exactly the same thing over and over.
Eventually, because the $addtocart string is growing every time through
the loop, PHP will run over its time limit (30 seconds by default in
php.ini) or if you turned that off (Bad Idea) you'll eventually have
$addtocart filling up *ALL* of your RAM and *ALL* of your swap and run
completely out of memory.
It's also quite possible that when all this code is boiled down into
machine language that it's so short and simple that PHP doesn't even get
an interrupt to stop this runaway process, and the 30-second rule will
never kick in. Course, you'll still run out of RAM eventually, though
that could take hours and you'll notice the performance drop and kill
PHP/httpd long before that happens. Hell, you'd reboot the machine if
nothing else.
So, what you REALLY want is probably more like:
if ($key != 'submit')
You only want to test each $key *ONE* time to be sure it's not the
'submit' button.
Another option might be to put this *before* the foreach loop:
unset($_POST['submit']);
That just plain gets rid of 'submit' before you start your loop.
It's kinda icky to alter what came in through $_POST directly, though, so
even better might be to do:
$inputs = $_POST;
unset($inputs['submit']);
foreach($inputs as $key => $value)
Also, *before* you do any of this stuff, you *REALLY* should have a
prepared list of what keys you expect to get in $_POST.
If you're going to walk through *all* of $_POST, blindly, and turn every
key/value pair into a variable in your script, you might as well just turn
register_globals back "On" because you've completely defeated that
security measure of turning it "Off" in the first place! Your security
measure of having register_globals "Off" is pointless if you're going to
circumvent that. :-)
Look at the example code of why register_globals was turned off in the
first place. If somebody sends POST data to your server with the same
hack in it, and you blindly walk all of $_POST turning every key/value
pair into a variable, you've got the exact same problem.
There's also no real need to write a loop of your own to do this:
http://php.net/extract
pretty much was designed to do exactly what you are doing. :-)
If you find yourself doing something you think everybody has done a
million times, check to see if there is a function for doing that :)
So, the final code you *SHOULD* use looks a lot more like this:
//Define valid input 'key's from the FORM:
$acceptable_keys = array('name', 'phone', 'email', 'submit');
//Get all the inputs (possibly bogus hacked ones) from POST data:
$inputs = $_POST;
//Find anything in the inputs that *IS* bogus:
$bogus_keys = array_diff(array_keys($inputs), array_keys($acceptable_keys));
if (count($bogus_keys)){
//RED ALERT!
//Somebody is trying to hack your site through your FORM
//*DO* something here.
//Record their IP.
//Log the bogus inputs and their values, maybe
//But logging them could *also* be a security hole. Hmmmm...
//Have a human review those logs routinely.
//Do whatever you need to do to close off your HTML tags and print
//a nasty anti-hacker error message.
exit;
}
//Now that you know all the stuff in $inputs is kosher:
//Get rid of 'submit', I guess, if you want...
unset($inputs['submit']);
//Set all the keys to their value:
extract($inputs);
PS
I don't think this line is strictly kosher:
\$$key = $value;
At least, I've never seen it done that way, and it wasn't in the manual as
kosher last time I checked. That's been awhile, though.
Not that it matters, since 'extract' is a better answer anyway :-)
--
Like Music?
http://l-i-e.com/artists.htm
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
