My general rule of thumb regarding variables from post and/or get, is such: if you use it once, dont throw it into a variable, if you use it more than once, then put it in a variable. If you name things consistently and well, regardless of how long from now you are reading the code, $_POST['password'] will be just that, and it's not any less obscure then $pass, especially if used just once, and cleaner... Regardless of the cost of performance decreasing, performance is an overall thing, if you dont care for performance in any one place, you don't really care for performance, and in the "instant" world that we live in, performance should be as serious of a consideration as security, that is actually why Facebook wrote their PHP interpreter, they understand that users want FAST. And performance means you should consider things, even overly extensive commenting, even if something is better done one way, doesnt mean it is the best way to do it. For example i LOVE recursive functions, but i never write them in scripting languages, because they run a lot slower then a for loop, however more elegant any such function would be, it just doesn't perform... And i understand it's a simple example, those two variables don't really matter, and wont use much more space, but constantly thinking consistency, security, performance, will help you achieve better code in the end, even if puristically-speaking it's worse. Another reason is overall clarity and clenliness of the code, counting lines is a bad practice, but avoiding unnecessary lines helps, and it adds up, sometimes using inline logic and avoiding declaring unnecessary variables goes a long way to make your code much more concise and readable actually, especially if you have a lot of it. That said, i always initialize my arrays, because it avoids notices... but here is a brief example: (!DEBUG) || error_log("Fetch Data: ".memory_get_usage()/1048576); reads and writes a lot better and faster then: if(DEBUG) { $memory = memory_get_usage()/1048576; error_log("Fetch Data: ".$memory); } if($_POST) is just that, that will check if someone/thing used POST to POST data to your script. You don't post anything else, and you check for existence of other variables, you are not any better with checking for submit. And your browser will most certainly never send a post request just for the kick of it, so... not sure what your objection to a cleaner if statement is exactly..? (It's as easy to pass a submit as it is to pass a username and a password, you dont gain any security by checking for submit) You want the path of the script as well, if i put it in my test folder under doc root, your action will never execute because it will be a level off. $self=htmlentities($_SERVER['PHP_SELF']); Servers occasionally mess up, and it can not even be their fault, php messes up, stuff happens. If you use security in layers, then code with a hashed password will not reveal your password, where as if i am able to dump your source, i have the keys to your kingdom otherwise. And you comment and document ways to get the hash, or provide a utility to generate that hash, through say an install script or something that will fill in the password. That code and the fact that apache should execute it, is currently your only layer of security, so make it two... Your scenario: server messes up or i change htaccess to dump your code i look at code i own keys to your kingdom, and you dont know about it My scenario: server messes up i look at code i'm still SOL... no keys, so your "protected" area is still protected.... -- The trouble with programmers is that you can never tell what a programmer is doing until it’s too late. ~Seymour Cray On Thu, May 19, 2011 at 8:57 PM, tedd <tedd.sperling@xxxxxxxxx> wrote: > At 2:29 PM -0400 5/19/11, Alex Nikitin wrote: > >> Also don't declare a bunch of needless variables for their one-time use, >> don't compare unsanitized strings with a binary unsafe operator, server >> variables contain link to current script, here are examples of what i >> mean: >> > > I object. > > First of all 'needless' is in the eye of the beholder. I've seen ton's of > 'needless' comments about how programmers waste precious space by declaring > needless variables because they can do things more cryptic. I've also heard > in the past how programmers should be cryptic and even shorten their > variable names, not use indenting, and do all sorts of other nonsense to > save space and make their code run quicker. > > However, they forget a couple of important considerations. > > 1. Code running tomorrow will run-faster and cost-less to store than today. > That's a fact and while we can argue, the argument becomes less important as > time passes. If I don't win this argument today, I will win it tomorrow. > > 2. I also claim that if I can make my code more readable and easier to > maintain by adding a 'needless" variable now and then, then it's well worth > the cost. And as I said before, that cost is reducing every day, while > maintaining readable code is becoming more important. So again, I'll > eventually win this argument. > > So, whenever you feel in the mood, create another 'needless variable' > because they need love too! > > > > -$self = basename($_SERVER['SCRIPT_NAME']); >> +$self = $_SERVER['PHP_SELF']; >> > > They return different things. I want the name of the script. > > ---------- > > -$submit = isset($_POST['submit']) ? $_POST['submit'] : null; >> -if($submit == 'Submit') >> >> +if($_POST) >> > > if($_POST) what? > > I'm cleaning the the POST variable. If the user has not clicked "Submit", > then I don't want to evaluate the POST. Sure, there are ways to forge and > pass a POST variable, but this is one step in cleaning a superglobal. > > --------- > > > -$pw = 'pw'; // define your password here >> -$user_id = isset($_POST['user_id']) ? $_POST['user_id'] : null; >> -$password = isset($_POST['password']) ? $_POST['password'] : null; >> -if (($user_id == $id) AND ($password== $pw)) >> >> +$pw='1a91d62f7ca67399625a4368a6ab5d4a3baa6073'; //sha1 hash of the >> password: php -r "echo sha1(\"pw\");" >> +if (@strcmp($id, $_POST['user_id']) == 0 && strcmp($pw, >> sha1($_POST['password'])) == 0) >> > > Sure. > > Here's the problem -- where's the novice going to get the hash for the > password? > > I don't want to force the novice into another step in this demo. > > Besides, the only way that an evil doer can see the code in text is *if* > there is a problem with the server -- isn't that right? If that's the case, > then there's more problems here than what the user could have planned for. > > However, if there is another way, please explain. > > > Cheers, > > tedd > > -- > ------- > http://sperling.com/ >