Edit database content with a web page

Question

I'm new to PDO, and I want to make sure I made this in the correct way. This way is working, but I have a feeling it's unclean.

I have this input:

$query = $handler->query('SELECT * FROM content WHERE id=1');
while($r = $query->fetch()) {
?>
<div id="content">
 <div class="container">
 <form action="<?php echo BASE_URL ?>admin/edit_process.php?edit_about" method="post" autocomplete="off" id="contact_form" name="contact_form" role="form">
 <div class="form-group">
 <label for="message">Izmeni sadržaj sa početne strane:</label>
 <textarea class="form-control" id="message" name="message" required="" rows="6"><?php echo $r['edit_about']; ?></textarea>
 </div>
 <div class="form-group">
 <button class="btn btn-success float-right" type="submit" name="submit">Sačuvaj</button>
 </div>
 </form>
 </div>
</div>
<?php
}

Then it goes to this:

if(isset($_REQUEST['edit_about'])){
 if(!$user->isLoggedIn()){ Redirect::to(''.BASE_URL.''); }
 if(!$user->hasPermission('admin')) { Redirect::to(''.BASE_URL.''); }
 if(Input::exists()){
 try{
 $message = $_POST['message'];
 $sql = "UPDATE `content` SET `edit_about` = :message WHERE `id` = 1";
 $query = $handler->prepare($sql);
 $query->execute(array(
 ':message' => $message
 ));
 Redirect::to(''.BASE_URL.'index');
 }catch(Exception $e){
 die($e->getMessage());
 }
 }
}

And this is the output:

<?php
$query = $handler->query('SELECT * FROM content WHERE id=1');
while($r = $query->fetch()) {
 echo $r['edit_about'];
}
?>

Answer 1

The biggest problem with your PDO code is error reporting. die($e->getMessage()); is the worst method possible. Citing from my article on error reporting (which I highly recommend to read):

• for the casual user the error message is pretty cryptic and confusing
• it it essentially useless for the site programmer/admin as they likely won't be browsing the site at the moment and will have no idea that error has been occurred
• the information provided in the error message is extremely useful for a potential hacker, as it provides them with the feedback for their actions and may leak some sensitive information about your site

So never write a try..catch operator unless you have a certain scenario to be executed in the catch block as it will be useless and harmful.

Another suggestion is just a matter of taste. Given you have only one variable in the query, you can use a positional placeholder instead of a named one, sparing yourself writing some extra code:

if(Input::exists()){
 $sql = "UPDATE `content` SET `edit_about` = ? WHERE `id` = 1";
 $query = $handler->prepare($sql);
 $query->execute([$_POST['message']]);
 Redirect::to(BASE_URL.'index');
}

As you may notice, the PDO code looks a bit different from the other code you are using. With a simple PDO wrapper could make this code even simpler and uniform:

if(Input::exists()) {
 $sql = "UPDATE `content` SET `edit_about` = ? WHERE `id` = 1";
 DB::run($sql, [$_POST['message']]);
 Redirect::to(BASE_URL.'index');
}

The example is taken from my PDO wrapper usage examples but you can use any wrapper you like.

Also, I don't know which framework you are using, but $_POST['message'] seems a bit alien to me. I would expect something like Input::get('message') instead.

• Monk: So i should use trigger_error($e->getMessage()); is that correct?
• Monk: can i remove try...catch and die(), do i need them? Can i just make redirection to 500 page?
• Your Common Sense: a redirection is a bad idea, as it makes no sense. a page that has an error returns a 300 code, and then a page it redirected to returns 500. A total mess. You should not redirect, you must return 500 right away.
• Monk: I'm little bit confused. Is it good to show error or not? Because in my code i gave you i had that error, and you suggest me the code where i don't have try...catch and error report. Is it safe to have it like that, without error report? If not, should i use then try...catch and put trigger_error($e->getMessage()); inside of it? Thank you.
• Your Common Sense: trigger_error($e->getMessage()); inside of it makes absolutely no sense. the outcome would be exactly the same as without try catch. Your other questions are already answered in the article I linked at the top of my answer.