Changing Mysqli to PDO connect

[phpnewbie34]

I have been reading and have been told that prepared statements are one way to prevent SQL injections.

My mysqli conect works, but my new PDO statement doesn't connect

$username = "root";
$password = "1212";
try {
$conn = new PDO('mysql:host=localhost;dbname=test', $username, $password);
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch(PDOException $e) {
echo 'ERROR: ' . $e->getMessage();
}

Can you tell me why this doesn't work?

Thanks in advance friends

[Weedpacket]

What do you mean "doesn't work"? Is your PC marching around outside waving a placard? Your code would echo an error message if it failed to connect, so what is that message (there's no point putting in debugging code if you don't use it when you're debugging)? What do some of the other properties of the PDOException object (such as errorInfo) say?

phpnewbie34 wrote:

I have been reading and have been told that prepared statements are one way to prevent SQL injections.

If that's the only reason you're changing, it's not a good enough reason: MySQLi also supports prepared statements.

[phpnewbie34]

I've heard from somebody else that using PDO is superior to mysqli prepared statements
by means of being faster and easier to write

I am now looking to transfer my Msqli code into PDO it is more secure

This is what I'm trying to convert now:

$query = mysqli_query($con,"SELECT * FROM pixs WHERE title LIKE '%$keyword%' OR Description LIKE '%$keyword%'");

Do you have suggestions?

[sneakyimp]

You could start by Reading the Friendly Manual.

[bradgrafelman]

phpnewbie34;11028933 wrote:

I've heard from somebody else that using PDO is superior to mysqli prepared statements by means of being faster and easier to write

As for the "faster", intuition would lead me to believe that the opposite is true (PDO is, after all, an abstraction layer whereas MySQLi is clearly a lot more specialized). I doubt the difference is really that significant one way or the other, though. (In other words, the decision to use PDO over MySQLi solely based on which one executes faster is probably shortsighted in scope and isn't even the best place to invest time improving efficiency.)

As for "easier to write," that, to me, is dangerously close to saying that green is the best color because it's my favorite. (In other words, look at the coding examples in the manual and form your own subjective opinion rather than relying on anyone else's.)

If you're in the market for subjective opinions, here's mine: Prepared statements are at best overkill (at worst: a waste) for SQL queries you don't plan on executing more than once.

phpnewbie34;11028933 wrote:

II am now looking to transfer my Msqli code into PDO it is more secure

Again, that's not a good reason to change. In fact, it's not even a correct reason; PDO is no more or less secure than MySQLi or even the old mysql extension. If you don't know how to properly sanitize the necessary data or properly use parameters in prepared queries, you'll be able to write software that contains vulnerabilities no matter which API you choose.

phpnewbie34;11028933 wrote:

This is what I'm trying to convert now:

$query = mysqli_query($con,"SELECT * FROM pixs WHERE title LIKE '%$keyword%' OR Description LIKE '%$keyword%'");

Do you have suggestions?

Assuming $keyword wasn't previously escaped, replace both instances of:

$keyword

with:

" . mysqli_real_escape_string($con, $keyword) . "

and the code will be "secure" as far as protecting against SQL injections/errors goes.

Otherwise, you'll have to be more specific as to what you're trying to convert that code snippet into. As sneakyimp points out, the manual pages for MySQLi and PDO functions/methods that execute queries or create prepared statements all contain code examples that augment the documentation of all of their parameters. For example, you could compare the code examples on the manual pages [man]mysqli.prepare[/man] and [man]pdo.prepare[/man] to get a rough idea of the code you'd be writing if you chose one API over the other.

[cretaceous]

bradgrafelman;11028939 wrote:

If you're in the market for subjective opinions, here's mine: Prepared statements are at best overkill (at worst: a waste) for SQL queries you don't plan on executing more than once.

Thank you for that informative comment Mr G.
Been feeling guilty every time I avoid them, that makes me feel a whole lot better..

[Weedpacket]

I tend to use them even for ostensibly "one-off" queries; partly because values are often provided as structures built elsewhere, and it's easy enough to traverse and bind. Also, a query that is "only used once" may in fact sometimes be called on at different times with different values depending on circumstances; caching statements as they are prepared seems to work. And sometimes the job of building the SQL is separate from the job of executing the statement. The method doing the former might as well return an instance of PDOStatement (or an instance of a subclass, or some other object that has a PDOStatement as a property) that is independent of specific values (plus I find out sooner if the built statement is broken). Even if I don't plan on executing it more than once.

But that's just my opinion 🙂 Really, it's whatever fits in best with the application's architectural principles.

[sneakyimp]

I'm not sure in what way you guys are suggesting that prepared statements are overkill -- and admittedly, I have not used PDO a lot - but I believe it's a bit easier to use a prepared statement often times than trying to properly construct the query string and match all your quotation marks and deal with string-related syntax and [man]mysqli_real_escape_string[/man] calls and all that nonsense. Imagine what this would look like if you had to construct the SQL string manually:

$str_insert_image_sql = 'INSERT INTO `images` (vid, container_cdn_uri, container_name, root_filename, filename_extension, is_default, time_added, original_width, original_height, remote_url_hash) VALUES (:vid, :container_cdn_uri, :container_name, :root_filename, :filename_extension, :is_default, :time_added, :original_width, :original_height, :remote_url_hash)';
$arr_insert = array(
  ':vid' => $cloudCarImage->vid,
  ':container_cdn_uri' => $cloudCarImage->container_cdn_uri,
  ':container_name' => $cloudCarImage->container_name,
  ':root_filename' => $cloudCarImage->root_filename,
  ':filename_extension' => $cloudCarImage->filename_extension,
  ':is_default' => $isDefault,
  ':time_added' => date('Y-m-d H:i:s'),
  ':original_width' => $cloudCarImage->original_width,
  ':original_height' => $cloudCarImage->original_height,
  ':remote_url_hash' => $cloudCarImage->remote_url_hash
);
$stmt = $this->db->prepare($str_insert_image_sql);
if ($stmt === FALSE){
  throw new Exception("INSERT images prepare statement FAILED: \n" . $str_insert_image_sql . "\n" . print_r($arr_insert, TRUE));
}

That would be 11 calls to mysqli_real_escape_string [or $db->real_escape_string()] and all of the quotation marks (single? double?) to boot. I'm fully aware that a shrewd programmer can auto-generate most of this sql by creating clever classes or looping through arrays of column names or something. I still think prepared statements can be slightly more expressive than old-school SQL construction.

[bradgrafelman]

sneakyimp;11028963 wrote:

Imagine what this would look like if you had to construct the SQL string manually:

$str_insert_image_sql = 'INSERT INTO `images` (vid, container_cdn_uri, container_name, root_filename, filename_extension, is_default, time_added, original_width, original_height, remote_url_hash) VALUES (:vid, :container_cdn_uri, :container_name, :root_filename, :filename_extension, :is_default, :time_added, :original_width, :original_height, :remote_url_hash)';
$arr_insert = array(
  ':vid' => $cloudCarImage->vid,
  ':container_cdn_uri' => $cloudCarImage->container_cdn_uri,
  ':container_name' => $cloudCarImage->container_name,
  ':root_filename' => $cloudCarImage->root_filename,
  ':filename_extension' => $cloudCarImage->filename_extension,
  ':is_default' => $isDefault,
  ':time_added' => date('Y-m-d H:i:s'),
  ':original_width' => $cloudCarImage->original_width,
  ':original_height' => $cloudCarImage->original_height,
  ':remote_url_hash' => $cloudCarImage->remote_url_hash
);
$stmt = $this->db->prepare($str_insert_image_sql);
if ($stmt === FALSE){
  throw new Exception("INSERT images prepare statement FAILED: \n" . $str_insert_image_sql . "\n" . print_r($arr_insert, TRUE));
}

No imagination required:

$str_insert_image_sql = sprintf("INSERT INTO `images` (vid, container_cdn_uri, container_name, root_filename, filename_extension, is_default, time_added, original_width, original_height, remote_url_hash) VALUES (%u, '%s', '%s', '%s', '%s', %u, NOW(), %u, %u, '%s')",
  $cloudCarImage->vid,
  $this->db->real_escape_string( $cloudCarImage->container_cdn_uri ),
  $this->db->real_escape_string( $cloudCarImage->container_name ),
  $this->db->real_escape_string( $cloudCarImage->root_filename ),
  $this->db->real_escape_string( $cloudCarImage->filename_extension ),
  $isDefault,
  // ':time_added' => date('Y-m-d H:i:s'), --> NOW()
  $cloudCarImage->original_width,
  $cloudCarImage->original_height,
  this->db->real_escape_string( $cloudCarImage->remote_url_hash )
);
$sql = $this->db->query($str_insert_image_sql);
if ($sql === FALSE){
  throw new Exception("INSERT images query FAILED: \n" . $str_insert_image_sql . "\n" . print_r($arr_insert, TRUE));
}

EDIT: Yes, I made some assumptions about the types of the columns/data in use, and yes, I realize that named parameters can be seen as an advantage with prepared statements. And yes, I still personally save prepared statements for use when I need to execute a query more than once... so sue me. 😉

[laserlight]

sneakyimp wrote:

That would be 11 calls to mysqli_real_escape_string [or $db->real_escape_string()] and all of the quotation marks (single? double?) to boot.

If you use sprintf instead of concatenation, it would not be all that different, except for the quotation marks and the fact that you'll be using format specifiers rather than placeholder names (but not everyone uses placeholder names anyway).

Security-wise, the advantage of a prepared statement is that the data is separated from the SQL statement itself, thus posing less of a risk that forgetting to do or incorrectly doing data sanitation will allow for an SQL injection, compared to say, directly embedding strings and other values. This can come at a cost in efficiency should the statement only be used once, but overall I judge it to be the better option, except for cases where there are no values to bind (and the statement is only run once).

[sneakyimp]

bradgrafelman;11028967 wrote:

No imagination required:

$str_insert_image_sql = sprintf("INSERT INTO `images` (vid, container_cdn_uri, container_name, root_filename, filename_extension, is_default, time_added, original_width, original_height, remote_url_hash) VALUES (%u, '%s', '%s', '%s', '%s', %u, NOW(), %u, %u, '%s')",
  $cloudCarImage->vid,
  $this->db->real_escape_string( $cloudCarImage->container_cdn_uri ),
  $this->db->real_escape_string( $cloudCarImage->container_name ),
  $this->db->real_escape_string( $cloudCarImage->root_filename ),
  $this->db->real_escape_string( $cloudCarImage->filename_extension ),
  $isDefault,
  // ':time_added' => date('Y-m-d H:i:s'), --> NOW()
  $cloudCarImage->original_width,
  $cloudCarImage->original_height,
  this->db->real_escape_string( $cloudCarImage->remote_url_hash )
);
$sql = $this->db->query($str_insert_image_sql);
if ($sql === FALSE){
  throw new Exception("INSERT images query FAILED: \n" . $str_insert_image_sql . "\n" . print_r($arr_insert, TRUE));
}

EDIT: Yes, I made some assumptions about the types of the columns/data in use, and yes, I realize that named parameters can be seen as an advantage with prepared statements. And yes, I still personally save prepared statements for use when I need to execute a query more than once... so sue me. 😉

Egad! The horror. 😉

I have immense respect for your knowledge and skill, bradgrafelman, and I'm not a particularly litigious person, so you needn't worry about a lawsuit. I know that I make a lot of mistakes when I have to keep my %s and %u straight and when I have to add quotes here but not there and also have to keep my real_escape_string calls straight and so on. I have this fantasy that I'll be able to write some kind of master database table abstraction class which takes care of generating all my sql for me for simple inserts, updates, deletes, and simple select queries. For a particular table in my database, I would extend this class and could quickly add records like so:

$x = new TableX(array(
  "column1" => "column1 value",
  "column2" => 42,
  "column3" => "column3 value",
  "column4" => "etc."
));
$x->createDBRecord(); // throws exception when there's a problem

// updates also easy
$x->column2 = 43;
$x->updateDBRecord();

// fetching records by id also easy
$x = TableX::fetchByColumn1("column1 value");
if (is_null((!$x)) die ("not found.");

I've made a couple of working prototypes of these classes and just LOVE the way that it generates my SQL for me. If I'm careful to insert my records using my createDBRecord and updateDBRecord methods, I can also keep my write queries separate from my read queries in the event I end up working with a master DB and slave DBs. I'm turning into an OOP zealot.

[sneakyimp]

laserlight;11028969 wrote:

If you use sprintf instead of concatenation, it would not be all that different, except for the quotation marks and the fact that you'll be using format specifiers rather than placeholder names (but not everyone uses placeholder names anyway).

Agreed, but you still have to be careful about the sequence of your format specifiers to make sure they match your list of column names (and the values you are supplying). This level of focus is something I find tiresome after long bouts of programming and am convinced the tedium of this vigilance is what leads to lots of code errors.

laserlight;11028969 wrote:

Security-wise, the advantage of a prepared statement is that the data is separated from the SQL statement itself, thus posing less of a risk that forgetting to do or incorrectly doing data sanitation will allow for an SQL injection, compared to say, directly embedding strings and other values. This can come at a cost in efficiency should the statement only be used once, but overall I judge it to be the better option, except for cases where there are no values to bind (and the statement is only run once).

This is no small advantage I think (see prior statement about the tedium of vigilance).

[Bonesnap]

I've been using the concatenation/real_escape_string method for a long time, so keeping track of quotes (single and double) and periods, etc. isn't much of an issue with me as it has become second nature, but I have started using prepared statements recently. I could go either way. Neither seems to be more difficult to follow or write than the other. For me at this point it's a personal preference and really I'm open to using either. One thing I don't care for when using MySQLi's prepared statements are the question marks. Doesn't really tell me anything at a glance; definitely prefer %s, %d, etc. instead, which you can get other ways.

I've often thought about writing a database abstraction class mostly for fun but also as a learning experience, but I never seem to have the time. C'est la vie.