Need to convert a small mysql script to PDO

Can someone convert this small script to PDO for me? I tried it on my own and it didn't work out. I just don't know exactly what needs to be used instead of mysql_real_escape_string or mysql_num_rows. Basically, if you can go through the whole thing and change it to PDO that will be great.

session_start();
if(isset($_SESSION['userid'])){
$user = mysql_real_escape_string($_SESSION['userid']); 


$favid = mysql_real_escape_string($_GET['favid']);


$query = mysql_query("SELECT * FROM ajaxfavourites WHERE user='$user'");
$favlimit = mysql_num_rows($query);


$query = mysql_query("SELECT * FROM ajaxfavourites WHERE user='$user' AND  favid='$favid'");
$matches = mysql_num_rows($query);


if($matches == 0 && $favlimit < 30){
mysql_query("INSERT INTO ajaxfavourites (user, favid, exptime) VALUES ('$user', '$favid', CURRENT_TIMESTAMP)");

echo "";
}


if($matches != 0){
mysql_query("DELETE FROM ajaxfavourites WHERE user='$user' AND favid='$favid'");

echo "";
}

} else {

echo "Invalid session!";

}

Code (markup):

 

One of the biggest features of both mysqli and PDO is that you no longer blindly paste values into queries. the ::query method is only for when you are not passing parameters but expect a result set. the ::exec method is only or when you are not passing values but do not expect a result set... what you want is :repare -- which is for any time you are passing values to the query.

There are two ways of structuring a query without values in it, using a "?" where a value would go so you can state them in order, or a label starting with a colon such as ":user" to use with either ::bindParam or to pass to PDOStatement::execute.

Let's take just your first query. Because with prepared statements values are passed separately from the query string, you no longer have to spend the extra statement sanitizing values, they're sanitized FOR YOU. I'm not sure why you'd be pulling that count first, without using COUNT(), so I'm going to change it to that. Remember if you just want the count, don't retrieve the entire result set, that's a waste of processing time and memory. Using count means we can use PDOStatement::fetchColumn to pull it further simplifying things.

// assumes $db is a connected PDO object

$statement= $db->prepare('
	SELECT COUNT(*)
	FROM ajaxfavorites
	WHERE user = :user
');
$statement->execute([
	':user' => $_SESSION['userid']
]);
$favlimit= $statement->fetchColumn();

Code (markup):

We "prepare" the query, then pass the value to :user in an associative array. (*note I'm using PHP 5.4+ style arrays). This handles the sanitation for you (more so if you disable "emulated prepares) and is how the entire system should have worked all along. Blindly dumping values into query strings is one of the most DUMBASS things ever invented and should NEVER have been allowed in the first place.

A full rewrite of that would go something like this -- I'd re-arrange things slightly so you aren't running queries you don't need to -- since on matches you wouldn't even be using favlimit. Remember, never do a if/condition on the same value more than once. If you checked for >0 first on matches, you wouldn't need to check for 0 on else.

session_start();
session_regenerate_id();

if (isset($_SESSION['userid'])) {

	$data['favid'] = $_GET['favid']
	
	$statement = $db->prepate('
		SELECT COUNT(*)
		FROM ajaxfavorites
		WHERE user = :user
			AND favid = :favid
	');
	$statement->execute([
		':user' => $_SESSION['userid'],
		':favId' => $_GET['favid']
	]);
	
	if ($statement->fetchColumn() > 0) {
	
		$statement = $db->prepare('
			DELETE FROM ajaxfavorites
			WHERE user = :user
				AND favid = :favid
		');
		$statement->execute([
			':user' => $_SESSION['userid'],
			':favid' => $_GET['favid']
		]);
		
	} else {
	
		$statement = $db->prepare('
			SELECT COUNT(*)
			FROM ajaxfavorites
			WHERE user = :user
		');
		$statement->execute([
			':user' => $_SESSION['userid']
		]);
		
		if ($statement->fetchColumn() < 30) {
		
			$statement = $db->prepare('
				INSERT INTO ajaxfavorites (
					user, favid, exptime
				) VALUES (
					:user, :favid, CURRENT_TIMESTAMP
				)
			');
			$statement->execute([
				':user' => $_SESSION['userid'],
				':favid' => $_GET['favid']
			]);
			
		}
		
	}

} else {

	echo 'Invalid session!';
	
}

Code (markup):

Untested, might be some typo's, but should be enough for you to get the idea.

Moving to PDO can be a big step if you've only ever dealt with mysql_ functions, but that's part of why I think we should have stopped TEACHING that broken insecure methodology a decade ago. Switching to prepare/execute's way of thinking is tricky as it's WAY different. It's also way better as it opens the door to things like POEM (Prepare once, execute mostly) like I showed in your other thread where I showed how to store things in the session instead of trying to track favorites in the database itself; which would likely be faster than this approach and as I said, makes this code obsolete/unneeded.

 

It didn't work. When you say "assumes $db is a connected PDO object" will this suffice as a db connection?

try {
    $db = new PDO(
        'mysql:host=localhost;dbname=some_db',
        'some_name',
        'password'
    );
} catch (PDOException $e) {
    die('Connection failed: ' . $e->getMessage);
}


session_start();
session_regenerate_id();

if (isset($_SESSION['userid'])) {

    $data['favid'] = $_GET['favid']
  
    $statement = $db->prepate('
        SELECT COUNT(*)
        FROM ajaxfavorites
        WHERE user = :user
            AND favid = :favid
    ');
    $statement->execute([
        ':user' => $_SESSION['userid'],
        ':favId' => $_GET['favid']
    ]);
  
    if ($statement->fetchColumn() > 0) {
  
        $statement = $db->prepare('
            DELETE FROM ajaxfavorites
            WHERE user = :user
                AND favid = :favid
        ');
        $statement->execute([
            ':user' => $_SESSION['userid'],
            ':favid' => $_GET['favid']
        ]);
      
    } else {
  
        $statement = $db->prepare('
            SELECT COUNT(*)
            FROM ajaxfavorites
            WHERE user = :user
        ');
        $statement->execute([
            ':user' => $_SESSION['userid']
        ]);
      
        if ($statement->fetchColumn() < 30) {
      
            $statement = $db->prepare('
                INSERT INTO ajaxfavorites (
                    user, favid, exptime
                ) VALUES (
                    :user, :favid, CURRENT_TIMESTAMP
                )
            ');
            $statement->execute([
                ':user' => $_SESSION['userid'],
                ':favid' => $_GET['favid']
            ]);
          
        }
      
    }

} else {

    echo 'Invalid session!';
  

Code (markup):

There was one typo I saw "prepate" instead of "prepare". I don't see anything else obvious.

 

Oh, I should probably show you how bindParam works...

$statement= $db->prepare('
	SELECT COUNT(*)
	FROM ajaxfavorites
	WHERE user = :user
');
$statement->bindParam(':user', $_SESSION['userid']);
$statement->execute();

Code (markup):

Bindparam can be really handy if you have multiple values to pass to the same query that are going to end up in the same variable. Let's say you already had the list of ID's pulled into an array from storing them in a session instead of the ajaxfavorites table... say $_SESSION['favorites'] is an array where the index is the favid and the value is the expiration time (which we won't use here).

$statement= $db->prepare('
	SELECT *
	FROM adverts
	WHERE favid = :favid
');
$statement->bindParam(':favid', $index);
foreach ($_SESSION['favorites'] as $index => $data) {
	$statement->execute();
	if ($row = $statement->fetch()) {
		// output the resulting row here
	}
}

Code (markup):

That's POEM in a nutshell, Prepare Once, Execute Mostly. As we 'bound' the $index variable to the :favid label, it will pull those values for us during the ::execute greatly simplifying what's inside the loop.

One of the many reasons PDO and mysqli (which operates in a similar fashion) is so vastly superior. That PDO can be used in more than just mysql AND can accept that array format to it's ::execute method IMHO swings the pendulum WAY in it's favor.

More so if you get into really advanced concepts like extending PDO to restrict it's functionality to 'named queries' and tighten up it's security a notch.

 

AMEN!

A corrollary to this is NEVER write the same code TWICE unless there is no other reasonable choice.

 

At a glance that looks correct... when you say it didn't work, what is it reporting for errors?

Oh, and I REALLY recommend doing the session start BEFORE the try/catch. That could even on a catch state output something which would prevent sessions from working. Like all other HTTP header type events, you're best off putting them in the code as early as you can. It's unlikely to be the cause of your problem or to even cause a problem, but it's worth doing as a matter of good habit.

There is one other possibility, and that's if you're on a winblows based server you'd need to use sockets with PDO instead of ports. I've only really ever encountered that on XAMPP though. For some reason this problem ONLY seems to exist in PDO which can make it a bit annoying.

 

It doesn't report errors. It just doesn't insert anything to DB.

 

Do you have full reporting on? Check the server logs to see if it's complaining there.

PDO can sometimes fail to report errors to PHP, so you have to check the logs. It's why I like to local test since it's easier to flush the logs to make new errors easier to spot.

Though it's a good idea to check the logs frequently anyways just to catch errors that might leave the system WORKING, but could break on a future PHP update or result in overfilled logs that can suck your server performance into the deepest pit of hell when the archive process kicks in.

 

Actually, I just enabled the mysqli extention via WHM, so I think I am going to do the same thing through mysqli. Seems to be a bit easier. I like PDO, but it's like a whole new world to me. I will try it on my own, if I can't I'll post another question here. Thanks for all your help!!

 

Well keep in mind using mysqli, that you should NOT be using the "function equivalents" as those are stop-gap for people who can't grasp objects that is likely to go away in future, and you should STILL be using prepared queries instead of blindly dumping your values into the query string. That separation of data from the query is one of the reasons mysql_ functions are going (well, technically now HAVE GONE) away.... the other big one being the ability to restrict scope in more complex systems as having the connection in global scope is made of /FAIL/

Prepare is just more of a pain in the ass as mysqli has no convenient "pass an associative array" implementation, so you HAVE to use bindParam... and it lacks label support so you're stuck with vague question marks in the query.

As such done PROPERLY using mysqli is even more complex...

<?php

session_start();
session_regenerate_id();

$mysqli = new mysqli(
	'localhost',
	'some_name',
	'password',
	'some_db'
);

if ($mysqli->connect_error) die(
	'Connect Error (' . $mysqli->connect_errno .') ' . 
	$mysqli->connect_error
);

if (isset($_SESSION['userid'])) {

	$statement = $mysqli->prepare('
		SELECT COUNT(*)
		FROM ajaxfavorites
		WHERE user = ?
				AND favid = ?
	');
	$statement->bind_param(
		'ii',
		$_SESSION['userid'],
		$_GET['favid']
	);
	$statement->bind_result($count);
	$statement->execute();
	if ($stmt->fetch() && ($count > 0)) {
		$statement = $mysqli->prepare('
			DELETE FROM ajaxfavorites
			WHERE user = :user
			AND favid = :favid
		');
		$statement->bindParam(
			'ii',
			$_SESSION['userid'],
			$_GET['favid']
		);
		$statement->execute();
		/*
			you might want to check $statement->affected_rows here to see if
			anything was actually deleted
		*/
		
	} else {
	
		$statement = $mysqli->prepare('
			SELECT COUNT(*)
			FROM ajaxfavorites
			WHERE user = ?
		');
		$statement->bindParam(
			'i',
			$_SESSION['userid']
		);
		$statement->bind_result($count);
		$statement->execute();

		if ($stmt->fetch() && ($count < 30)) {
	
			$statement = $mysqli->prepare('
				INSERT INTO ajaxfavorites (
					user, favid, exptime
				) VALUES (
					?, ?, CURRENT_TIMESTAMP
				)
			');
			$statement->bindParam(
				'ii',
				$_SESSION['userid'],
				$_GET['favid']
			);
			$statement->execute();
			
		}
	
	}

} else echo 'Invalid session!';

Code (markup):

Yet another reason I favor PDO... Since if you are pulling some stupid stunt like this:

$statement = $mysqli->query("SELECT * FROM ajaxfavourites WHERE user='$user'");

Code (markup):

You'd be doing it ALL WRONG in a "security, what's that" kind of way.

 

Didn't know you posted something... I posted too, then removed. Let me try what you posted there.

 

Happens sometimes if we both hit post at about the same time, or you were typing while I was typing and posting. No biggy.

Be warned again, may be typo's and that's untested, but should be sound in concept. As I favor PDO it might not be the best mysqli as I use the latter far less -- To the point I had to refer to php.net to make sure I had the syntax right -- maybe we could get someone more experienced with that to chime in?

I know the general concepts of how it's supposed to work, I just don't use it often enough to remember how to dot the t's and cross the i's... or.. uhm, yeah.

 

Thanks again for all your help. I've been at the computer all day long today. I may get back to it tomorrow. I think partially why I can't make it work is because I'm so pooped right now. Been at my desk for over 11 hours. Need a break.

 

Ooph, yeah at 11 hours you probably should have gotten up for the day three hours before. With programming anything past about seven hours a day seems to become diminishing returns.

When I'm working on big projects I like to set a timer and take a 15 to 30 minute break every two hours whether I need it or not. Sometimes even when you THINK you're on fire, that break can give you time to mull over possible complications or trigger a "eureka" moment even if you're not thinking about it. Some of the best breakthroughs in comprehension comes when you aren't focused on the task -- PARTICULARLY when learning. The brain takes time to digest information into something meaningful -- that's why we have dreams when sleeping is that's a side-effect of the processing.

I learned that lesson the hard way a few decades ago, sometimes when you're stuck on understanding something or going "why isn't this working" your best bet is to simply back away from the problem and come at it fresh later.

It's why even in my current retirement I keep two or three plates spinning at once, working on something else can change your perspective when you come back to a project.

Though I'm weird, I can't concentrate if I don't have distractions in the background, and I rarely see success if I'm only working on one thing at a time.

 

I didn't look at the code... I have no idea whether it is lengthy or short... but as a general practice if you need to convert a PHP script to use PDO instead of mysql functions there's usually a bunch of other crap going on in that script that needs to be "converted". Generally it's easier to rewrite legacy code than "convert".