Welcome Guest, Not a member yet? Register   Sign In
Handling bad input data
#1

[eluser]Unknown[/eluser]
Hello.

I have a function in controller which returns information about the company in json format.

Here is the code:

Code:
public function getCompanyInfo() {
  if (!$this->input->get('company_id')) {
             exit;
  }
  
  $company_info = $this->companies_model->getCompanyInfo($this->input->get('company_id'));
  
  if (count($company_info) == 0) {
      exit;
  }
  
  $this->output->set_content_type('application/json');
  $this->output->set_output(json_encode($company_info));
}

I'm really confused with the right way to handle bad input data. There are several situations, for example there is no company_id in get params, or (in other function) the data is not valid at all (for example - not valid email).
In these cases I want to return some http error code (like 400 - bad request), and some json informatio (like {"error":"bad_input"}). Not sure though if returning 400 is a good solution, but another guy who works with javascript/ajax told me that there are some javascript functions which handle http errors like 400 and it's a good thing to return 400 with json explanation.

My first question is: is it ok to return 400 + json info as a solution to handle bad input data? Now I have only exit to prevent errors or hack attempts.
My seconf question is: if I have a function, where there are a lot of places where I should interrupt execution of function, and return some headers with json, how would I do that? set_content_type, set_headers, set_output works if I put them in the end of function, but for example in my case above I want something like:
Code:
if (!$this->input->get('company_id')) {
   $this->output
                             ->set_status_header('400');
        ->set_content_type('application/json')
        ->set_output(json_encode(array('error' => 'bad_input')));
                        exit;
  }

and the same in
Code:
if (count($company_info) == 0) { ... }


(in case the ID is wrong so the model won't return anything)


Could you explain how to deal with such situations in a good maner, I have a feeling that my code is a mess.

And one more thing: how could I use instead of $this->output->... my own function for all controllers? for example:
Code:
function bad_input() {
   $this->output
                             ->set_status_header('400');
        ->set_content_type('application/json')
        ->set_output(json_encode(array('error' => 'bad_input')));
                        exit;
}
If I put it in a helper it won't work because of "$this". But If I decide to rename 'error'=>'bad input data from user' I'll have to do that in several controllers which is not good Smile

I'll appreciate any advices, thanks in advance, and sorry for my poor english Smile



#2

[eluser]aquary[/eluser]
To return 400, I think it's OK, but might not for this case.

Quote:The Web server (running the Web site) thinks that the data stream sent by the client (e.g. your Web browser or our CheckUpDown robot) was 'malformed' i.e. did not respect the HTTP protocol completely. So the Web server was unable to understand the request and process it.

IMO, I don't feel like your case fall in this type... Your data is "good" HTTP wised, but "bad" in the logic level. I'd return only "error" with an error message. But as I said, IMO :-)

2 Question: you can put the bad_input function into MY_Controller, then it'll be available via $this->bad_request() in every controller extended from it. Read the userguide about creating MY_Controller, and it'd be easy to create one yourself. Though, I suggest you put make it as a protected function instead.
#3

[eluser]Unknown[/eluser]
Thanks!

But still I have the biggest problem, when I need to send something (400 or just JSON error) in several parts of my code. What I want to have is something like this:

Code:
public function getCompanyInfo() {
  if (!$this->input->get('company_id')) {
   // Here some output data
   $this->output
        ->set_status_header('400')
        ->set_content_type('application/json')
        ->set_output(json_encode(array('error' => 'bad_input')));
         exit;
  }
  
  $company_info = $this->companies_model->getCompanyInfo($this->input->get('company_id'));
  
  if (count($company_info) == 0) {
    // And here.
    $this->output
        ->set_status_header('400')
        ->set_content_type('application/json')
        ->set_output(json_encode(array('error' => 'bad_input')));
         exit;
  }
  
  $this->output->set_content_type('application/json');
  $this->output->set_output(json_encode($company_info));
}

The output functions (set_output for example) works only if you put them in the end of functions, but what If i want to stop my program and send output before the end of function?

And any other ideas about if the idea of 400 error is a good or bad solution would be appreciated Smile

aquary, I agree with you, I'd return only json data or just die; because normally an ordinary user won't send wrong data because there will be a lot of validation in javascript. So if php side get invalid data it would mean that this is a hack attempt, so I dont want to send any infromation, just die.
But still not sure...
#4

[eluser]meigwilym[/eluser]
You really don't need to set a 400 error here.

The situation is that a GET array element is missing. If you're writing the url links yourself then this is up to you, or the link could be opened with the element deleted.

You'd be better off creating an error/can't find/404 view, then returning this with a 404 code. Or, rethink your app and perhaps show a paginated list of all companies.

Mei




Theme © iAndrew 2016 - Forum software by © MyBB