I cam across a post that was actually a small tutorial on CodeIgniter. The tutorial had what ever it requires to write a basic controller. I am picking up the same post and adding my comments to the post.

The original post – http://phpmaster.com/untangling-mvc-with-codeigniter/

The controller

<?php public function index() { $this->load->helper("form"); $this->load->library("form_validation"); $this->form_validation->set_rules("first", "First Name", "required"); $this->form_validation->set_rules("last", "Last Name", "required"); $this->form_validation->set_rules("email", "Email Address", "required|valid_email"); if ($this->form_validation->run() == false) { $this->load->view("phpmasterform_view"); } else { $first = $_POST["first"]; $last = $_POST["last"]; $email = $_POST["email"]; $data = array("first_name" => $first, "last_name" => $last, "email" => $email); $this->load->model("phpmasterform_model"); $this->phpmasterform_model->insert_address($data); $this->load->view("formsuccess"); } }

What is wrong with this Controller?

The controller doesn’t utilises the full powers of a framework. Any decent framework or a develop methodology will suggest you to move configurations to a separate place. CI has configuration settings for almost everything. The configuration folder is easily identifiable and path is /application/config . In this folder is file autoload.php . This file is commented and is self explanatory. I will move the form helper and form_validation library loading tasks to autoload.php . Read more about it here, Auto-loading Resources. Then in same config folder I will create another file named form_validation.php which will hold all my validation rules. This will be a centralised place to hold all the validation rules. I can easily modify just one file and change validation rules on my whim and fancy. Read more about this here, “Saving Sets of Validation Rules to a Config File“. All these were just small tips and one can read the manual and find more about them. But the biggest mistake I found in this controller was blindly consuming the user input. The commandment of web development says

Thy shalt never trust user input.

CI provides a Input Class library. This library is so important that CI loads it by default. So if you are using CI, use this library.

This was not the only possible security hole. If run through other files in CI, they all start with following line

if ( ! defined('BASEPATH')) exit('No direct script access allowed');

.

This restricts some one from directly accessing the file if path to that file is known.

After all this the controller is changed to following

<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed'); public function index() { if (false === $this->form_validation->run(feedback)) { $this->load->view("phpmasterform_view"); } else { /* if you have set following "$config['global_xss_filtering'] = TRUE;"in your application/config/config.php then you don't need to pass 2nd param as true in following three lines. */ $first = $this->input->post("first", true); $last = $this->input->post("last", true); $email = $this->input->post("email", true); $data = array("first_name" => $first, "last_name" => $last, "email" => $email); $this->load->model("phpmasterform_model"); $this->phpmasterform_model->insert_address($data); $this->load->view("formsuccess"); } }

Following is form_validation.php , this will go in application/config/ folder.

<?php if ( ! defined('BASEPATH')) exit('No direct script access allowed'); $config = array( 'feedback' => array( array( 'field' => 'first', 'label' => 'First name', 'rules' => 'required' ), array( 'field' => 'last', 'label' => 'Last name', 'rules' => 'required' ), array( 'field' => 'email', 'label' => 'Email', 'rules' => 'required' ) ), );