Archives

PHP Style Guide

In order to ensure clean, readable, and reusable code, it’s necessary for every project or organization to adopt a set of rules for writing and editing code. Ringfree is no different, thus I present the official Ringfree PHP Style Guide.

Environment

The recommended editor for use within Ringfree is Atom. Atom is cross platform, intuitive, highly extensible, and open source. Additionally Atom was designed to work very well with version control systems. Use of the autoclose-html package is recommended for front end work.

Hard Rules

There are a few hard rules associated with writing PHP code within Ringfree:

  1. Spaces rather than tabs. Atom will manage this in such a way that the two are essentially seamless, but always use spaces. Please convert any files found not using spaces.
  2. Use an indent size of 4 spaces. Using fewer can lead to code bleeding together on larger projects, using more is simply unnecessary. Please convert any files found not using a 4 space indent size.
  3. Do not use filler whitespace. If a line has no content, do not indent it to indent level of the current code block. Doing so simply wastes space. Saving a file in Atom will, by default, strip any filler whitespace.
  4. Save files with a trailing newline. Most editors should either do this by default or have an option for this.
  5. Skip the closing ?> tag on pure PHP files. Using it is unnecessary and can sometimes cause issues in older PHP versions.
  6. Use a minimum of 2 newlines between class and function declarations and other blocks of code in the global space.
  7. Use a minimum of 1 newline between blocks of code within a function or class declaration. The specifics of which will be outlined below.

Block Structure

Please define classes, functions, loops, conditionals, etc with the opening bracket on the same line. This is in contrast with ANSI C standards, but ultimately saves some space and makes for cleaner looking code. For example, the following is good:

function ringfree_action() {
    // Some code...
}

And the following should be avoided:

class RingfreeClient
{
    // class declarations...
}

Additionally you should always use brackets for loops and conditionals. Another good example:

if ($x == 1) {
    // Some code...
}

And the following should be avoided:

if ($x == 1)
    // Some code...

PHP is not strict about whitespace and while Ringfree doesn’t have any hard rules about implementing one-liners, they should generally be avoided unless in a block of code requiring multiple successive one-liners. So with a single one-line conditional, the following is preferred:

if (false === $atdata) {
    exit("You must specify a valid XML file.\n");
}

And the following should be avoided:

if (false === $atdata) { exit("You must specify a valid XML file.\n"); }

However with multiple successive one-liners, the opposite is preferred.

 Statement and Block Grouping

Statements and code blocks should be grouped and separated in a logical, sensible fashion. Blocks should never immediately precede or follow statements and all statements of notably different purpose should be delimited in some capacity (normally empty lines). The best way to illustrate is an example of what not to do:

$client->id = $id;
$client->name = $name;
$client->desc = $desc;
$clients[] = $client;
if (count($clients) < 10) {
    continue;
}

In the above bad example we have four statements followed by a conditional containing a fifth statement. Note how there is not visible indication that the function of the code effectively changes in a couple of places. The following is an example of how to group and delimit the above:

$client->id = $id;
$client->name = $name;
$client->desc = $desc;

$clients[] = $client;

if (count($clients) < 10) {
    continue;
}

Now we can clearly identify the code in three groups:

  1. Assignment statements.
  2. Append statement.
  3. Conditional one-liner.

In the case of more complex function/class declarations and loop/conditional statements, it’s necessary to delimit the first contained statement from the parent statement/declaration:

function sortThings($things) {

    $people = array();
    $stuff = array();

    foreach ($things as $thing) {

        if ($thing instanceOf Person) {

            $people[] = $thing;
            continue;
        }

        $stuff[] = $thing;
    }

    return array($people, $stuff);
}

Note that for basic language constructs like continue and break we don’t necessarily need to group them individually. It’s normally quite obvious what they do and everything should be well commented in the first place.

Commenting

The general rule of thumb here at Ringfree is to always err on the side of verbose commenting. Not only does the practice benefit new people reading the code, but it helps put things in perspective while writing it. Every line doesn’t need a comment but in general it’s good practice to comment every block and group of statements. Here’s an example of proper commenting on the last example:

// Sort an array into nested arrays of Person objects and other variables.
function sortThings($things) {

    // Declare two empty arrays to store our variables.
    $people = array();
    $stuff = array();

    // Iterate through our $things array and sort into the above arrays.
    foreach ($things as $thing) {

        // Test if the variable is a Person object.
        if ($thing instanceOf Person) {

            // If so, append to the $people array and continue the loop.
            $people[] = $thing;
            continue;
        }

        // If not a Person object, append to the $stuff array.
        $stuff[] = $thing;
    }

    // Return an array containing our sorted $people and $stuff arrays.
    return array($people, $stuff);
}

Make a note to always add your comment at the same indent level as the code you’re commenting. When in doubt, add a comment even if it seems to state the obvious. Other people will be looking at your code and what’s obvious to you might not be obvious to them. Also use some common sense with your comments and actually describe what’s happening in the code. Take the following example:

// Adds a new PBX to the database.
function addPbx($ctid, $host, $ipaddr, $customer, $node, $dbserver) {
    // Some code...
}

Note that the comment before the function declaration tells us what the function does, but doesn’t actually tell us what’s happening or how to run the function. A better example:

/*
Adds a new PBX to the database and returns the CTID upon success and
NULL upon failure This function takes 6 arguments:
    1 - $ctid - Integer: the container ID of the PBX.
    2 - $host - String: the hostname (url) of the PBX.
    3 - $ipaddr - Integer: the IP of the PBX converted to a long int.
    4 - $customer - Integer: the customer ID to assocaite this PBX with.
    5 - $node - String: the URL of the host node.
    6 - $dbserver - String: the URL of the PBX's database server.
*/
function addPbx($ctid, $host, $ipaddr, $customer, $node, $dbserver) {
    // Some code...
}

Note how the above comment gives us precise information for how to use the function along with what sort of return value we can expect. Note that every last function doesn’t need every last thing detailed, but with longer and more complex function, doing so helps out quite a bit.

Also note that longer multi-line comments should be enclosed in the expected /* and */ tags while shorter comments should be prefixed by the expected // single line comment symbol. There are no hard rules as to when one should be used over the other but use common sense. A few lines of single commenting is fine, 15 lines of single commenting should obviously be replaced with a multi-line comment.

Naming Things

A loose rule for naming things here at Ringfree is to use Pascal Case for class declarations, and Camel Case for function and variable names. Take the following example (comments omitted for brevity):

class RingfreeClientSite {

    private $siteName;

    public function getSiteName() {
        return $this->siteName;
    }

    public function setSiteName($name) {
        $this->siteName = $name;
    }
}

Please try to avoid using short, single word names in the global scope in order to prevent polluting it with duplicate or non-descriptive names (though doing so is fine within a function or class declaration with appropriate commenting).

Coding Style

With PHP, in general procedural code is more efficient than equivalent object-oriented code and not substantially less efficient than equivalent functional code. The overall readability of a procedural versus object-oriented solution to a problem largely depends on the problem itself whereas a functional solution is almost always more difficult to read.

That said, please avoid functional coding techniques in your PHP code. Use iteration instead of recursion; use named functions instead of anonymous ones; don’t be afraid of mutable data; etc. While PHP is more or less capable of working within the functional programming paradigm, it’s primarily implemented as a procedural and object-oriented language and, at least within Ringfree, will be used as such.

As to when to implement a procedural versus an object-oriented solution, please try to err on the procedural side on smaller projects due to the reduced overhead, but don’t go to extreme lengths to avoid objects or refuse to use otherwise useful libraries as a result. Also be aware of when objects offer superior performance (such as in situations requiring complex nested data structures). With larger projects, objects will likely (but not always) be necessary to keep the project organized.

Non-Compliant Code

If you find PHP code not in compliance with the rest of this style guide, please open a bug report indicating the name of the project, the file name, the affected lines, and a description of why the code is not in compliance. If the project is assigned to you, please resolve style related bugs in their own commits.

Hacked extension checklist

1) [via the cli] On all PBX nodes (node001 and node002), run the call count script to see has an abnormally high amount of calls active.   Scammers treat a compromised extension as a sip trunk so you’ll see one extension having 50 +/- calls up.  By comparison, our heavy use customers generally only have 20-30 tops (asheville womens)

Here’s what a normal day might look like:

# sudo /rfbin/ringfree-showcallcount 
1900056  fourseasonscfl-001.ringfree.biz     10 active calls

Here’s what a hacked extension on fourseasonscfl would look like:

# sudo /rfbin/ringfree-showcallcount 
1900056  fourseasonscfl-001.ringfree.biz     64 active calls

2) [via the web gui] Log into that PBX’s pbxadmin interface.  Go to the cdr and identify which extension has been hacked.  It will be immediately apparent as you’ll see the same extension making calls within seconds of each other.

3) [via the web gui] Change the SIP secret for the hacked extension.  Submit and Apply Changes.

4) [via Sansay] You have a good two minute period where call sessions and the hacked extension’s registration will still be shown in the Sansay.  Go to Monitoring > Subscriber Stats.  Filter by PBX hostname.  Generally single office companies will all be coming from the same Source IP.  Make a note of all registration Source IPs for the hacked extension.  If there are many extensions you can further filter by extension.

5) [upstream provider] Hacked extensions can drain balances very quickly which will halt traffic until it’s paid.  If you do not have access to the upstream carrier, find David, John or Molly.  It is imperative that the outbound carrier isn’t shut off for long due to a fraud alert suspension.

6) [determine attack vector by accessing customer’s router and calling customer] Next is to determine attack vector.  The most common method used to compromise extensions is by exploiting the extension’s SIP endpoint.  This occurs by someone gaining access and identifying the endpoint’s web configuration interface during an automated scan.  If you’ve ever opened up a port to access a customer’s web interface on their Polycom, you are opening them up to this form of attack which is why you should always remove the port forward when done (or preferably, use a screen sharing application to access the phone without opening it up to the web).

Less commonly, but what occurred on Monday, is a phone that has been hooked up directly to the internet via a modem where in most topologies you would usually place a router with a firewall.  This presents the phone’s easily compromised web interface directly to the internet where it will be exploited and credentials stolen.

7) [on p.ringfree.biz] grep the hacked extension’s Source IP that was not shared from all available /var/log/xferlog* files.  It is important to verify that the ftp server wasn’t compromised.  If you see that the hacker’s ip has access the ftp server, and you’re sure the IP you’re searching for did not originate from a customer’s office or location, turn off the ftp service until it can be further analyzed.  Do not do this unless you get a match.  It’s not a common attack vector and has occurred only once with an ftp account whose username and password was set up in matching numerics only.

[root@ppt / ]# service vsftpd stop

8) [with the customer] Bringing the phone back online should not be done until a) the SIP secret has been changed, b) port forwards to that phone’s registration were check to be disabled and c) the phone has been verified not to be registering over the internet without a firewall in place.

The majority of this can be done within 5 minutes within first notification of account suspension.