Inserting a role using PDO

Question

My question has to do with :

(a) Can this be made more secure?

(b) Are there best practices in relation to setting timezone?

(c) What best approach should be taken defining use of utf8?

        $my_success = 0;
        $my_message = '';
        $my_id = 0;

        try {

            # declare & set variables using $my_env
            list ($db_dbdriver, $db_hostname, $db_database, $db_username, $db_password) = Connection::dbConnect($my_env);

            # set default timezone
            date_default_timezone_set('America/Los_Angeles');

            # create & set connection
            $db_options = array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8');
            $conn = new PDO($db_dbdriver . ":host=" . $db_hostname . ";dbname=" . $db_database . ";charset=utf8", $db_username, $db_password, $db_options);
            $conn->exec("set names utf8");
            $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            $conn->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

            # Prepare an SQL statement
            $my_sql = 'INSERT INTO Role (Role_Name, Role_CDate, Role_CBy, Role_EDate, Role_EBy) 
                VALUES (:Role_Name, NOW(), :Role_CBy, NOW(), :Role_EBy)';
            $stmt = $conn->prepare($my_sql);

            # bind parameters to prevent SQL Injection
            $stmt->bindValue(':Role_Name', $role_name);
            # defaults to Now()...
            $stmt->bindValue(':Role_CBy', $role_cby);
            # defaults to Now()...
            $stmt->bindValue(':Role_EBy', $role_cby);

            # execute the SQL statement
            $stmt->execute();

            # closes the cursor, enabling the statement to be executed again
            $stmt->closeCursor();

            # grab ID value of the new record
            $my_id = $conn->lastInsertId();

            # close & unset the connection
            $stmt = null;
            unset($stmt);
            $conn = null;
            unset($conn);

            # Set success value to TRUE
            $my_success = 1;
            $my_message = 'success';

        } catch(PDOException $e) {

            # Catch error message
            $error_message = $e->getMessage();

            # Append error message to error log
            # ...

            # Set success value to FALSE & return an error message
            $my_success = -1;
            $my_message = $error_message;
            $my_id = 0;

        }

Answer 1

There are several points which can be improved, and by the style of your code I'm assuming that you are using this project to learn OOP..?

First off, your questions:
a) Since you are not outputting anything to the browser, or anything but an SQL server, this code has very few possible attack vectors. You are using prepared statements, which means that you have correctly protected yourself against SQL-injections.
That is, as long as you've ensured that the strings are indeed valid UTF-8, before sending them over to the DB server. This is especially important if these strings comes from a browser, and not the code itself.

b) Only two, really. Always set the timezone, and make sure it's set to the same timezone everywhere (and every time) you create a new time/date object. The timezone can/should be changed after creation, if it's necessary to change it.

c) You seem to have taken the appropriate steps, at least in the code posted above. I've found a site listing the general steps for UTF-8 support in PHP scripts, which might be useful for you.

That said, here are some points that could be improved in the code posted:

1. First off I'd change the line retrieving the DB variables, from your $my_env array(?). I would rather implement a DB connection factory here, or even a singleton if it's a small enough project. (Or to learn singletons.)
2. You've set the timezone using the old date_*() functions, which aren't really recommended any more. I strongly recommend looking into DateTime class family for date-time related needs.
3. Even though you've set the timezone, the code you've posted doesn't actually use it. You've used SQL's NOW() function instead, which uses the DBMS' timezone. This can be an issue if the PHP code and the database server are using different timezones, and you're later on using PHP to manipulate/compare timestamps.
4. You don't need to close the cursor, unset variables or anything like that. PHP normally does a perfectly good job of cleaning up afterwards, and it might even be detrimental to prematurely doing "cleanup". Normally this is only done when you have a specific use-case which requires you to clean up early.
5. You have what is essentially three return values, to communicate whether or not the query succeeded. You only need one, which should be either "0" for not OK, or the ID for OK. Any error messages or such should be added to an error log, by a dedicated function for this.
6. This one ties to the above, as the code should really be made into a function/method. This will allow you to maintain the code a lot easier, and make things quite a bit more flexible.
7. The try-catch block encompasses a lot more code than it needs to, which makes it a bit harder to follow the control-flow than necessary. One should only encompass the functions that actually can throw exceptions, if one doesn't just let them bubble up to a layer which can handle them in the proper location in the code-flow.
8. The connection class you have seems to have nothing to do with neither databases or connections to do, in fact it doesn't seem to following the OOP principles either. A method that simply takes input and returns output, without altering or depending upon the state of the object, isn't OOP. It just normal procedural programming, which is better done with pure functions. ;)

Here's a quick reorganization of your code, to give an example of the points I've listed:

function add_role ($name, $cby) {
    try {
        // Create or retrieve the database connection
        $conn = DB_Factory::create ($my_env);

        // Prepare an SQL statement
        $my_sql = 'INSERT INTO Role (Role_Name, Role_CDate, Role_CBy, Role_EDate, Role_EBy)
                    VALUES (:Role_Name, NOW(), :Role_CBy, NOW(), :Role_EBy)';
        $stmt = $conn->prepare ($my_sql);

        // bind parameters to prevent SQL Injection
        $stmt->bindValue(':Role_Name', $role_name);
        // defaults to Now()...
        $stmt->bindValue(':Role_CBy', $role_cby);
        // defaults to Now()...
        $stmt->bindValue(':Role_EBy', $role_cby);

        // execute the SQL statement
        $stmt->execute();
    } catch (PDOException $e) {
        // Catch error message
        $error_message = $e->getMessage();

        // Append error message to error log
        // ...

        // Set success value to FALSE & return an error message
        return 0;
    }

    // grab ID value of the new record
    return $conn->lastInsertId();
}