1

I need some ideas to make my code a bit cleaner for a pretty simple application with a simple database. I have the following idea but it's not working quite well. This is what I'm doing:

I have many queries on a page (takes up a lot of space and source code looks messy. I'd like to have an index of them in another PHP file that I could call via an include.

Except I ran into a problem. I have a query that looks like this on the external php page.

Queries.php:

$RegisterQuery = sprintf("INSERT INTO `testdb`.`users` (
                                                           `user_id`,
                                                           `username`,
                                                           `password`,
                                                           `First Name`,
                                                           `Last Name`,
                                                           `email`)
                     VALUES (
                             NULL,
                             '%s',
                             '%s',
                             '%s',
                             '%s',
                             '%s'
                              );",
                     mysql_real_escape_string($username),
                     mysql_real_escape_string($password),
                     mysql_real_escape_string($first_name),
                     mysql_real_escape_string($last_name),
                     mysql_real_escape_string($email));

The problem with this is that even when I don't query $RegisterQuery with mysql_query() it throws a bunch of MySQL warnings about using the mysql_real_escape_string() and about how it can't connect. I don't get how it's doing this when I'm not even querying the database!

Then again, I could easily ignore the warnings...

I'm just not sure I'm doing this properly in which case I mainly want to ask if there's a better way of doing this or should I just ignore the warnings?

Your feedback is greatly appreciated! Thanks in advance.

4 Answers 4

3

I'd suggest using parameterized queries (via for example mysqli or PDO).

You don't have to escape the strings if they are parameters to the query. This both makes the code clearer and also makes it easier to write secure code as it is a lot less likely you will leave an SQL injection vulnerability if you consistently use parameterized queries.

4
  • +1 Each call to mysql_real_escape_string is a hit on the database server. Parameterized queries are the standard solution. Oct 12, 2010 at 18:26
  • Where can I learn more about parameterized queries? I will be honest and say that I've never heard of them. Google search returns nearly nothing. =(
    – Tek
    Oct 12, 2010 at 18:51
  • @Tek: Check out stackoverflow.com/questions/60174/…
    – Mark Byers
    Oct 12, 2010 at 18:58
  • Ok, that answers how to do queries a bit more properly. But it doesn't really answer how it saves source space. Would you mind showing me a short example how this could save me space? I'm sure other people could use the example as well.
    – Tek
    Oct 12, 2010 at 19:17
2

The function mysql_real_escape_string requires a database connection. From php.net:

A MySQL connection is required before using mysql_real_escape_string() otherwise an error of level E_WARNING is generated, and FALSE is returned. If link_identifier isn't defined, the last MySQL connection is used.

If no database connection exists, it will return false, hence no escaping is done and the returned value isn't what you expect.

1

I think you need to rethink your solution. While you might think it's "messy" to have those queries in the source code, a future maintainer of that code is going to pull his hair out when he has to switch to a different file each time he wants to examine the database logic at a specific point.

As for your errors with mysql_real_escape_string, the function uses the MySQL server because it needs to escape the string with respect to the character set of the active DB connection.

0

mysql_real_escape_string makes a call to the database server.

Instead of calling mysql_real_escape_string when you're building the query, call it just before you execute the query.

However, ideally, you should use parameterized queries.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.