3

I have been using PDO for a couple of years now but I have never fully researched when you should prepare and execute using try and catch.

My understanding is that you should use try and catch when data may contain user input.

So this code for example is safe:

public function getDetails($filename, $what){
    $query = $this->handler->prepare('SELECT * FROM videos WHERE v_fileName = :v_fileName');
    try{
        $query->execute([
            ':v_fileName' => $filename
        ]);
    }catch(PDOException $e){
        return $e->getMessage();
    }
}

$filename in this example is something which comes from the URL.

When not getting anything from the URL for example like this it is also completely save:

$query = $this->handler->prepare('SELECT * FROM videos WHERE u_id = :u_id ORDER BY v_id LIMIT :climit,1');
$query->execute([
    ':u_id'     => $this->user->getChannelId($userid),
    ':climit'   => $optional[1]
]);

$fetch = $query->fetch(PDO::FETCH_ASSOC);

Is my understanding of preparing statements correct and if not, how should I do it?

Dharman
  • 30,962
  • 25
  • 85
  • 135
Tom
  • 606
  • 7
  • 28

2 Answers2

5

Only when you have a very good reason to do so.

This doesn't apply to only PDO exceptions. The same goes for any exception. Only catch the exception if your code can recover from it and perform some other action instead.

Catching exceptions just to echo or return $e->getMessage(); is not a valid reason. Your code doesn't recover from the problem, you are just handicapping the exception.

A good example of when you might want to recover is if you are using database transactions and in case of failure, you want to rollback and do something else. You can call PDO::rollBack() in your catch and then make your code perform some alternative logic.

Try-catch is not a security measure. It has nothing to do with user input. It is used only in situations when you expect your code to fail, but you have a plan B to handle the situation.

For more information, you can read My PDO Statement doesn't work and the article PHP error reporting

Dharman
  • 30,962
  • 25
  • 85
  • 135
  • I usually only use this when I am editing, when it goes into production I change it to an error message saying something went wrong. – Tom Jan 28 '21 at 18:54
  • No, that is what your error handler is responsible for. Your error handler should catch all exceptions and errors, log them to a file, and display a nice 500 error page to the user. This is not specific to PDO logic in any way. – Dharman Jan 28 '21 at 18:56
  • Why should and error handler be responsible for that? What is wrong or unsafe about an error message? – Tom Jan 28 '21 at 19:00
  • It's not unsafe. It's just a very very messy code. Instead of having an error message for every single line of code, you can have a single global error handler that handles all errors. The user doesn't need to know what exactly is broken, that is what error logs are for. You can find some more information here https://stackoverflow.com/questions/32648371/my-pdo-statement-doesnt-work and in this article https://phpdelusions.net/articles/error_reporting – Dharman Jan 28 '21 at 19:03
  • It would be unsafe if you left `ini_set('display_errors',1);` in production code. Make sure that your production system never displays internal errors to the user. – Dharman Jan 28 '21 at 19:05
  • I know but thanks for reminding me as I had actually forgotten. – Tom Jan 28 '21 at 19:06
1

Is my understanding of preparing statements correct and if not, how should I do it?

You use prepared statement to avoid SQL INJECTION. Prepared statements will quote the parameters to avoid it.

My understanding is that you should use try and catch when data may contain user input

The try catch block is used to handle erros in your application, not really related to prepared statements.

  • Can't SQL injection only happen with user input? If that is the case then the way I do it is fine right? – Tom Jan 28 '21 at 18:59
  • @Tom No, SQL injection happens when you mix PHP variables with SQL string. When you use parameter binding there should be no risk of SQL injection. – Dharman Jan 28 '21 at 19:01
  • My question wasn't about parameter binding though, it was about when to use `try` and `catch`. – Tom Jan 28 '21 at 19:03
  • @Dharman Correct me if I'm wrong but, variables set in PHP (that is NOT from user input) is not at risk of (malicious) SQL injection (unless, of course, the string the author set in PHP is itself malicious)? – GrumpyCrouton Jan 28 '21 at 19:33
  • @GrumpyCrouton Maybe, but I wouldn't trust any variable. By definition their value changes so you can't ensure that it is safe to be infected directly. It's easier to just bind all variable input instead. – Dharman Jan 28 '21 at 19:44