Review my prepared statement

Hi all,

I’ll jump straight to it. I’ve read prepared statements are the best method now when having to deal with a database; SELECT, UPDATE, etc

Now looking at my code below, what additional things should I add to the code to make it ‘secure’?

The get variable will always consist of something like this; “title-of-article”…

function displayArticleMetaDescriptionPrepared(){
	$conn = new mysqli('', '', '', ');
	$title = $_GET['title'];
	
	$sql = "SELECT blog_meta_descrip FROM blog WHERE blog_url = ? LIMIT 1 ";
$stmt = $conn->prepare($sql); 
$stmt->bind_param("s", $title);
$stmt->execute();
$result = $stmt->get_result();
while ($row = $result->fetch_assoc()) {
   echo $row['blog_meta_descrip'];
}
	

}

I look forward to the feedback, and pointers…

Well written functions should accept all inputs as call-time parameters and return the result they produce to the calling code. This is so that you can see (or search for) the affect of a function call at the point of the call, without needing to know or remember what the logic in the function is, and so that the result can be used in different contexts, not just for echoing on a web page.

A function is not responsible for creating a database connection. That is the responsibility of your main application code. Your main application code should create one database connection and supply it as an input to any function/class that needs it.

You must always have error handling for statements that can fail. For database statements that can fail - connection, query, prepare, and execute, the simplest way of adding error handling, without adding logic at each statement, is to use exceptions for errors and in most cases simply let php catch and handle the exception, where php will use its error related settings to control what happens with the actual error information (database statement errors will ‘automatically’ get displayed/logged the same as php errors.) You would set the error mode to exceptions in the connection code.

You should trim, then validate all inputs before using them. If this get input doesn’t exist, or is an empty string after being trimmed, your code should never get to the point of calling code to execute a query that requires the input. You should instead setup and display an error message for the user.

How is a tile the same thing as a url? You should use the same name for any particular piece of data throughout your code and name it for what it is.

Don’t repeat table names as part of each column name. This is just a waste of typing.

Since the url (or is it the title) column should be unique, it should be defined as a unique index in the database table. Then, there’s no point in using limit 1 in a query. The database will only find the one row of data, as fast as possible, via the index. There’s also no point in looping to fetch data from a query that will match at most one row. Just fetch the single row of data.

If you switch to the much simpler and more modern PDO database extension, about half of the database statements will go away. Also, the mysqli stmt get_result() method is not portable between different server builds. If you don’t manage your own php installation, there’s no guarantee that this method will exist, breaking your code. The PDO extension has no gotya’s like this.

htmlentities() should be applied to any dynamic value when it gets output in a html context to help prevent cross site scripting.

If you make the suggested changes, your function would look like -

function getArticleMetaDescriptionByTitle($pdo,$title)
{
	$sql = "SELECT meta_descrip FROM blog WHERE title = ?";
	$stmt = $pdo->prepare($sql); 
	$stmt->execute([ $title ]);
	return $stmt->fetchColumn();
}

Excellent, thank you. I’ll read through this and take note and get back to this because i’ll probably miss something. Thank you for taking the time to write a detailed reply, much appreciated.

Sponsor our Newsletter | Privacy Policy | Terms of Service