This morning, I wrote a quick-and-dirty table-revision generation script: (gist). It is intended to be run in the command line. Here's the entirety of it (149 lines):

/**
 * Creates a revision table for an already-existing table.
 * TODO: Better error checking/handling
 * TODO: Sanitize table name input
 *
 * Usage:
 * [user@host]$ php MySQLRevision.php
 * // Table name: <enter table name>
 *
 * The MIT License (MIT)
 * 
 * Copyright (c) 2014 Chris Forrence
 * 
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 * 
 * The above copyright notice and this permission notice shall be included in all
 * copies or substantial portions of the Software.
 * 
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 * SOFTWARE.
 * 
 */
const DB_TYPE = 'mysql';
const DB_NAME = '';
const DB_HOST = '';
const DB_USER = '';
const DB_PASS = '';

try {
    $dbh = new PDO(DB_TYPE . ":dbname=" . DB_NAME . ";host=" . DB_HOST, DB_USER, DB_PASS, array(PDO::ATTR_PERSISTENT => true));
}
catch(PDOException $e){
    $dbh = null;
}
if($dbh == null) {
    echo 'Could not get connection to database. Exiting.' . PHP_EOL;
    exit();
}

echo '// Table name: ';
$tn = trim(fgets(STDIN));

if(strlen($tn) > 0) {
    $toRet = array();
    list($query, $columns) = generateRevisionTable($dbh, $tn);
    if(is_array($columns) && count($columns) > 0) {
        $toRet[] = 'DROP TRIGGER IF EXISTS `rev_' . $tn . '_trigger_i`;';
        $toRet[] = 'DROP TRIGGER IF EXISTS `rev_' . $tn . '_trigger_u`;';
        $toRet[] = 'DROP TRIGGER IF EXISTS `rev_' . $tn . '_trigger_d`;';
        $toRet[] = 'DROP TABLE IF EXISTS `rev_' . $tn . '`;';
        $toRet[] = $query;
        unset($query);
        $toRet[] = 'DELIMITER ;;';
        $toRet[] = generateInsert($tn, $columns);
        $toRet[] = generateUpdate($tn, $columns);
        $toRet[] = generateDelete($tn, $columns);
        $toRet[] = 'DELIMITER ;';
    }
    else {
        $toRet[] = '// Could not find columns';
    }
    foreach($toRet as $q) {
        echo PHP_EOL . $q . PHP_EOL;
    }
}
$dbh = null;
unset($dbh);

function generateRevisionTable($dbh, $tn) {
    try {
        $q = $dbh->prepare("SHOW COLUMNS FROM $tn");
        $q->execute();
        $toRet = array();
        $table_fields = $q->fetchAll();
        $c = 'CREATE TABLE `rev_' . $tn . '` (' . PHP_EOL;
        $cl = array();
        foreach($table_fields as $t) {
            $c .= '    `' .  $t['Field'] . '` ' . ($t['Type'] == 'timestamp' ? 'datetime' : $t['Type']) . ',' . PHP_EOL;
            $cl[] = $t['Field'];
        }
        $c .= "    `revision_action` ENUM('INSERT', 'UPDATE', 'DELETE') NOT NULL," . PHP_EOL;
        $c .= "    `revision_id` INTEGER UNSIGNED NOT NULL AUTO_INCREMENT," . PHP_EOL;
        $c .= "    `revision_modification` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," . PHP_EOL;
        $c .= "    PRIMARY KEY(`revision_id`)" . PHP_EOL . ");";
    }
    catch(PDOException $e) {
        echo $e->getMessage() . PHP_EOL;
        $dbh = null;
        exit();
    }
    return array($c, $cl);
}

function generateDelete($tn, $cl) {
    $c = "CREATE TRIGGER `rev_${tn}_trigger_d` BEFORE DELETE ON `${tn}`" . PHP_EOL;
    $c .= " FOR EACH ROW BEGIN INSERT INTO rev_${tn} (";
    foreach($cl as $cls) {
        $c .= '`' . $cls . '`, ';
    }
    $c .= 'revision_action) VALUES (';
    foreach($cl as $cls) {
        $c .= 'OLD.`' . $cls . '`, ';
    }
    $c .= "'DELETE'); END;;" . PHP_EOL;

    return $c;
}

function generateUpdate($tn, $cl) {
    $c = "CREATE TRIGGER `rev_${tn}_trigger_u` AFTER UPDATE ON `${tn}`" . PHP_EOL;
    $c .= " FOR EACH ROW BEGIN INSERT INTO rev_${tn} (";
    foreach($cl as $cls) {
        $c .= '`' . $cls . '`, ';
    }
    $c .= 'revision_action) VALUES (';
    foreach($cl as $cls) {
        $c .= 'NEW.`' . $cls . '`, ';
    }
    $c .= "'UPDATE'); END;;" . PHP_EOL;

    return $c;
}

function generateInsert($tn, $cl) {
    $c = "CREATE TRIGGER `rev_${tn}_trigger_i` AFTER INSERT ON `${tn}`" . PHP_EOL;
    $c .= " FOR EACH ROW BEGIN INSERT INTO rev_${tn} (";
    foreach($cl as $cls) {
        $c .= $cls . ', ';
    }
    $c .= 'revision_action) VALUES (';
    foreach($cl as $cls) {
        $c .= 'NEW.`' . $cls . '`, ';
    }
    $c .= "'INSERT'); END;;" . PHP_EOL;

    return $c;
}

From this, I have a few questions:

  1. I know that I'm not handling my PDO object very well. For truly one-off scripts, is this acceptable, or should I create an internal PDO wrapper?
  2. It is currently very possible to inject code. Since this would (presumably) not be public-facing, is it something that I can ignore (and fix it as a low priority)?
  3. Am I missing anything in the script that would cause resource leaks, major problems, etc.?
  • The only real comment I have to add is, If you know you could do better then why not? For question 1 I would say its never acceptable to half do the job. 2) NEVER ignore a security threat. It is low risk, but there is still risk. – SaggingRufus Jan 7 '14 at 19:23
  • @SaggingRufus: Valid points; it comes down to time usage, and I think that as is, it is working code. Fixable? Certainly. Production code? Nope. For point 2, unfortunately, it looks like I'll have to use a blacklist to restrict characters (and then something to add an extra ` to entered table names). – Chris Forrence Jan 7 '14 at 20:40
up vote 1 down vote accepted
  1. I know that I'm not handling my PDO object very well. For truly one-off scripts, is this acceptable, or should I create an internal PDO wrapper?

It is generally discouraged to wrap PDO. Please see this excellent post on the topic which explains that better than I could.

  1. It is currently very possible to inject code. Since this would (presumably) not be public-facing, is it something that I can ignore (and fix it as a low priority)?

Why do you feel that you are less likely to inject dangerous code than 'the public'? Would you paste anything in there that you got in an email? Not even next year? Not even by accident?

  1. Am I missing anything in the script that would cause resource leaks, major problems, etc.?

You should probably set charset=utf8, and make sure that PDO::ATTR_PERSISTENT does what you think it does. You might be more interested in $q->errorInfo() than $e->getMessage(). Also, I would rewrite this:

if(strlen($tn) > 0) {
    ..lots of code here..
}

As this:

if(strlen($tn) > 0) {
    echo "Please provide input blah blah\n";
    exit();
}

..lots of code here..

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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