Internet Bandaid   [RSS Feed]

Archive for March, 2014

Human readable text belong in database, views or language files – Best Practice

without comments

My response to a developer after I saw human readable text in model and controller files:
============
Your human readable error messages need to appear in the view file. Right now i see code like this in controller or model files:

$errMsg = “Birth date can’t be in the future”;
$view = load(‘template.php,’,$viewdata = array(‘error’=>$errMsg));

Human readable text should only be found in view files, language files and the database (database only if it’s user generated content).

Consider the following scenario:
We hired a journalist/writer to help us edit the text on our website. It only takes us maybe a few hours to teach the journalist how HTML works…it’s just open tag, text, then close tag. Now the journalist can go into the View folder, and start helping us fix spelling mistakes and everything.

If the journalist also had to go into Model or COntorller folder to modify text, they are going to be intimidated by “alien” language and risk breaking everything.

So try putting your error messages directly in the view and toggled on/off by if statements like

if($errorLog['weight']) echo ‘the error message’;

directly in the view file

Written by John Lai

March 28th, 2014 at 4:30 pm

Posted in Uncategorized

Use Javascript to Render Email After Page Load – Best Practice

without comments

I looked on the website’s about us page and i see that everyone’s email addresses are EXPOSED. In other words, I see bad things like <a href=”mailto:someone@website.com”> in the view source.

This means that a spider/crawler can parse out everyone’s email addresses and spam them.

The better thing to do is something like

<a data-helloworld=”someone|||website.com”>

Then use javascrpt after page load to convert that to a proper mailto link.

Spiders/crawlers generally won’t trigger javascript

Written by John Lai

March 26th, 2014 at 9:19 am

Posted in Uncategorized

Code Details – Best Practice

without comments

1) USE PREPARED STATEMENTS/PARAMETER BINDS

In your model, I see you doing something good by sanitizing input before binding to a query:

$result=mysqli_query($GLOBALS['dbcon'],"SELECT * FROM tbl_automobiles where automobile_id=".((int)$id))

To make things easier, mysqli (and PDO db library) has something called prepared statements that let’s you bind parameters. I’m going to use some pseudocode right now to demonstrate:

$stmt = $PDO->prepare("SELECT * FROM tbl_automobiles where automobile_id = :id AND model LIKE :somemodel");
$stmt->bind('id',$_GET['id'], PDO::PARAM_INT);
$stmt->bind(':somemodel','%mercedes%',PDO::PARAM_STR);
$result = $stmt->execute();

Again, this is pseudocode so my syntax might be wrong. But hopefully you get the idea that you can have “place holders” for variables in your prepared statement, then use a bind() function to assign the appropriate values and value types to it. This is one of the common ways to prevent SQL Injection, which you are probably aware of (if not, do a google search on it, because it’s short but important read).

So for your assignment, try to incorporate mysqli’s prepared statements when you need to pass parameters to your queries.

2) PHP SHORT HAND ECHO NOTATIONI see you use PHP short hand echo notation like <?=$somevariable ?> are sometimes turned off on our customer’s servers, which means the page would give a parse error. We usually have no control over how customer servers are set up, so we should stick with <?php echo $somevariable; ?> to be safe.

The fact that you used PHP shorthand notation is a sign that you’ve done programming before, as such a feature is available in most templating languages. It’s just a shame that not every server has it enabled by default (probably because LAMP servers often render many serverside scripts like php, python, perl etc… and apache might not know if <?= $something ?> is a php, perl or python variable.)

3) SINGLE QUOTES vs. DOUBLE QUOTES – I see you used single quotes here like this <table border=’1′> for html attributes, but everywhere else, you use double quotes. Generally, we should pick one format and stick to it. We’ve been use to using double quotes

4) SANITIZE OUTPUT WITH htmlspecialchars() – I see code like <input value=”<?php echo $_POST['body_content']; ?>” />. This is very dangerous because if $_POST['body_content'] has the following value

"/><script type="text/javascript" language="javascript">location.href="http://virus.com";</script><input type="

Then a user will be redirected to a virus website. You should instead have code like this

<input value=”<?php echo htmlspecialchars($_POST['body_content']); ?>” />

To encode dangerous html characters. You can also htmlspecialchars() all variables prior to binding to the view.

5) Use Hyphen instead of Underscore in file names for SEO- instead of find_a_car.automobile.php, try something like find-a-car-automobile.php . If these pages are ever exposed to the Google search, then search engine optimization (SEO) practices is to use hyphen to denote spaces between words. This enables google to better understand what your page is about, improving visibility in google search result pages.

Written by John Lai

March 18th, 2014 at 8:30 am

Posted in Uncategorized

No SQL in the controller – Best Practice

without comments

Just reviewed someone’s code and I saw this

class Controller
{
function load()
{
    $result = $this->db->query("SELECT * FROM tbl_automobiles");
    $this->view->load('view.php',$result);
}
}

My Response was:
in your controller I see SQL statements. Generally, SQL statement should belong entirely in the model. Anything related to the database or grabbing data, keep that all in the model. So what you could do is Extend your db manage and call it something like AutomobileDbManager, and then have a function like getList() that has your SQL queries. Another good reason to do this is that in larger applications, you have a specialized person working on each tier of the application. You have a DB expert working only on model, and he should not have to touch your controller code. The PHP developer doing purely controller work. And front end person (or even data entry person) do only html in the view. A PHP developer should not have to see a single line of sql code, because he/she may not know it.

Written by John Lai

March 16th, 2014 at 1:38 pm

Posted in Uncategorized

Less PHP code in template – Best Practice

without comments

I was reviewing code written by a new team member and wrote a suggestion on how to improve. He wrote a view file that looked like this:

// listView.php
<?php

class vehicleListView
{

  function render($list)
  {
    echo "<table border='1'>
    <tr>
    <th></th>
    <th>ID</th>
    <th>Model</th>
    <th>Weight(kg)</th>
    <th>Year</th>
    </tr>";
    foreach($list->getData() as $vehicle)
    {
      $this->vehicleRender($vehicle->getData());
    }
    echo "</table>";
  }

  function vehicleRender($vdata)
  {
    echo "<tr>";
    echo "<td>". "<a href=\"/automobile-edit.php?id=" . $vdata['automobile_id']."\">edit</a>" ."</td>";
    echo "<td>" . $vdata['automobile_id'] . "</td>";
    echo "<td>" . $vdata['car_model'] . "</td>";
    echo "<td>" . $vdata['weight'] . "</td>";
    echo "<td>" . $vdata['manufacture_year'] . "</td>";
    echo "</tr>";
  }
}

?>

This was my response to him:

I see that your listView.php (which is a view) is structured as a class with functions that echo ‘<html>content</html>’. This should change because we often work with other specialized front end coders (or journalists who do data entry into the front end code) who do not know PHP and are scared by too much echo statements, php functions, class declarations etc…. Specialized front end coders only know html, css and “maybe” javascript.

So what you should do is set up your view in such a way that it hardly has any php code. In fact, try to keep your php code to only

  • if statements that are only 1 level deep as opposed many nested if statements
  • for loops (but try not to nest too many for loops in if statements and vice versa)
  • echo statements that only echo a maximum of 1 line as opposed to entire chunks of html
  • include statements to get other templates
  • simple content formatting functions like number_format(), substr(), and maybe even count()

Here is one suggestion, but there are plenty of other ways to do it as well. For example:

// templates/header.php
<html>
<body>

// templates/footer.php
</body>
</html>

// templates/list.php
<table>
<?php foreach($dataset as $row) : ?>
<tr>
<td><?php echo $row['column1']; ?></td>
</tr>
<?php endforeach; ?>
</table>

//listView.php
function someobjectfunction()
{
$content = '';
$dataset = getsomethingfrommodel();
ob_start();
include('templates/header.php');
include('templates/list.php');
include('templates/footer.php');
$content = ob_get_clean();
return $content;
}

This is one of the “popular” ways our team has been doing it lately. But i’m sure there are plenty of other ways to do it if you want to explore.

If you study other open source platforms like WordPress, Magento, Drupal, Code Igniter, etc…, you will notice that their view files are often held in a separate folder with MINIMUM php code … basically it tries its best to show straight html code. Sometimes the view files will have it’s own “templating” language like smarty, or aspx, etc…

So in the end, try to make your view files as friendly as possible for people who know only html and css.

Written by John Lai

March 15th, 2014 at 8:56 am

Posted in Uncategorized