joepie91's Ramblings

Note: This is a repost of a thread that I originally wrote on LowEndTalk. I can no longer support the operational and advertising policies of LowEndTalk, and have therefore decided to move the post here.

Okay, there have been quite a few threads here about questions regarding PHP, and every time I see people making the same mistakes in their code. Considering there are probably quite a few people here that use LEBs for development stuff, I'll point out some good and bad coding habits, with solid reasoning as to why you should or should not do it, for each of them. Unless you can give a valid counterargument that proves the reasoning invalid, you do not have an excuse not to do shit.

As a first word, I'd like to point out something: Please stop letting your ego get in the way of writing proper code. Stop making up reasons as to why you wrote code a certain way ("that's by design", "but that isn't necessary because ...", etc.) just to not have to admit that your code quality was bad. If there is an issue with your code, then get over it, fix it, and learn from it. I expect others to tell me when my code sucks, and the other way around, I expect other developers to genuinely consider suggestions for their own code. The reason I point out code sucks is not to boost my own ego, but because I care about proper and secure code being written. There is simply too much crap code going around, even in many popular and commercial software packages.

Note that this thread should not be considered a complete guide, nor will I touch on subjects like sanitation - there are plenty of other guides for this. This is more about safe code syle than anything else.

Things you should be doing

Understand your code.

Try to understand your code. When looking at your code, you should be able to pick any line in your code and know exactly what data is in what variable, accurate to the byte. If this isn't clear, make your code clearer. Don't be afraid to use more variables to make it more obvious what your code does, variables are cheap.

Why? By understanding your code, you will be able to spot bugs and vulnerabilities just from looking at it. It will take some time to get skilled at this, but eventually every single bug will be blatantly obvious to you, even if you're looking at code you wrote a year ago.

Indent your code.

Always, always, always indent your code properly. That includes PHP, that includes HTML, that includes CSS, that includes every single language that allows you to indent code. "It's not public code" is not a valid reason not to indent.

Why? Properly indented code allows you to see the structure of your code at a glance, and improves readability. Improved readability and understanding of code results in less bugs and less vulnerabilities. You should be able to understand what your code is doing at any point, just by looking at it, this makes it easier.

Use proper spaces.

While I can't tell you what your coding style should be, I do have some suggestions that would be a good idea idea to follow, to improve readability of your code.

$variable_name = some_function($something_else, $another_thing);

Leave spaces around assignment and equality operators, this makes it clear what part of your code belongs together. Don't add unnecessary spaces around braces, this will only make it less visible what belongs to what.

Why? This makes it obvious what part of your code belongs to what, and what the relationship between various elements is.

Use clear variable names.

Don't abbreviate variable names. There is no valid reason to do so. Make it clear from the name of the variable what's inside it. $unm is not a valid variable name. $username is a valid variable name, as it makes it obvious what is in it. $username_of_the_logged_in_user is far too long and makes your code hard to read; you could've used the much shorter $username.

Why? It allows you to see at a glance what a variable does. If you need to go to another line to see what a variable contains, you're doing it wrong.

Use proper braces.

Never ever ever leave out the curly braces around a code block. I don't care if it's one line or not, do not ever leave them out. Always enclose code blocks that belong to control structures in braces. Preferably, give each brace its own line as it makes it obvious what opening brace belongs to what closing brace.

Imagine you have this code:

if($confirm_delete === true)
     delete_item($_GET['id']);

Oops! The delete_item() function expects an integer, not a string. Well, let's just solve it like this, right?

if($confirm_delete === true)
     $id = (int) $_GET['id'];
     delete_item($id);

Whoops, you didn't realize that you weren't using braces, and now the item gets deleted regardless of whether $confirm_delete is true!

Why? People make mistakes, especially when in a rush. Don't think that "you wouldn't ever make the mistake above', because you will, at some point. Don't take the risk and just be safe and enclose it in braces; there is not a single reason not to do so.

Use curly braces for inline variables.

This trick isn't very well-known, while it should be. Say that you're using a variable inside a string, like so:

$whatever = "Hi, $something!";

Instead, do this:

$whatever = "Hi, {$something}!"

Why? There are many reasons to do this. 1. It will ensure that any characters that come after the variable name are not taken into account when parsing. 2. It makes it more obvious that a variable is a variable, which makes it easier to spot mistakes in things like queries. 3. Consistency. When using an array item in a code block, you enclose the key in apostrophes. To do the same in a string, you need to use curly braces. Logically, it makes sense to then enclose both array items and regular variables in apostrophes.

Stuff you really shouldn't be doing

Obfuscating your code.

No, obfuscating will not secure your code against 'hackers'. All it will do is make your code harder to read and understand - which actually makes it more likely that new vulnerabilities are introduced that you don't notice.Security through obscurity is not security.

Using exec() to do anything other than launch processes.

It's pretty likely that you can do whatever you want to do, with a PHP function. Don't use exec.

Other tips

Use a code editor.

While you can technically use notepad or nano to edit your code, it's not a very good idea - it doesn't do auto-indentation, and doesn't color-code your code, which makes it harder to spot mistakes.

A few suggestions for light-weight code editors: Windows: Geany, Notepad++, Sublime Text Linux: Geany, Vi,Sublime Text OS X: Sublime Text

Not sure if your code is good?

Post a code sample below, and I will read it, suggest improvements, give the reasoning behind those suggestions, and if applicable, update this initial post.

Something unclear?

If it's not clear to you why I am recommending a certain practice, or you think the reasoning isn't solid, then by all means point it out! I will attempt to clarify and take into account your concerns, and update this post accordingly.

Thank you for reading.