2

I am trying to rewrite all my queries to a PDO format. Currently i am trying to rewrite this function, but i cant seem to get it to work.

mysql_query function

    function checkLogin() {

    $this->sQuery = "SELECT * FROM users 
          WHERE gebruikersnaam='" . mysql_real_escape_string($_POST['gebruikersnaam']) . "'
          AND wachtwoord = '" . sha1($_POST['wachtwoord']) . "'";


    $this->rResult = mysql_query($this->sQuery)
            or die("Er is iets misgegaan " . mysql_error());


    if (mysql_num_rows($this->rResult) == 1) {  // login name was found            
        $this->aRow = mysql_fetch_assoc($this->rResult);
        $_SESSION['gebruiker'] = $this->aRow['voornaam'];

        header("location: dashboard.php");
    }
}

and this is how far i've come with the PDO:

       function checkLoginPDO(){
    $connect = new PDO(host, username, password); // Database Connectie maken (De host, username & password zijn in de config.php aan te passen)
    $sql = "SELECT * FROM users 
          WHERE gebruikersnaam='" . mysql_real_escape_string($_POST['gebruikersnaam']) . "'
          AND wachtwoord = '" . sha1($_POST['wachtwoord']) . "'"; 
    $value = $connect->prepare($sql); //Een variabele aanmaken die de PDO vast houdt. Vervolgens word de code voorbereid door de prepare functie
    $value->execute(); 
    if(mysql_num_rows($value->fetch()) == 1){
        $_SESSION['gebruiker'] = $row['voornaam'];
        header("location: dashboard.php");
    }
}

What am i doing wrong/forgetting?

Thanks in advance!

  • `mysql_real_escape_string()` should have been removed during your conversion process. – George Feb 27 '13 at 09:47
  • I removed the real_escape_string function but the after loggin in, i still get a blank screen! – Michael Koelewijn Feb 27 '13 at 09:49
  • Check the error logs. – Srisa Feb 27 '13 at 09:50
  • Matter of fact I've just noticed more mysql_* functions. If you are converting to PDO then you shouldn't be seeing any mysql_*. Also, if you've made changes to your actual code, please update it here so you don't get any repeat answers. – George Feb 27 '13 at 09:51
  • ( ! ) Fatal error: Uncaught exception 'PDOException' with message 'invalid data source name' in C:\wamp\www\pvb\resources\functions.php on line 44 ( ! ) PDOException: invalid data source name in C:\wamp\www\pvb\resources\functions.php on line 44 Which is: $connect = new PDO(host, username, password); Strange.. because those are correctly defined. – Michael Koelewijn Feb 27 '13 at 09:52
  • Read the manual, http://www.php.net/manual/en/pdo.construct.php – Srisa Feb 27 '13 at 09:54
  • (because you're a dutch man, you can read [this great tut](http://phptuts.nl/view/27/) too...) – Wouter J Feb 27 '13 at 09:56
  • I found it, thank god! my defined 'host' value was 'localhost'. It had to be mysql:host=localhost;dbname=dbname ofcourse! – Michael Koelewijn Feb 27 '13 at 09:57

4 Answers4

2

It should be something like this:

function checkLoginPDO(){
    $connect = new PDO(host, username, password); // Database Connectie maken (De host, username & password zijn in de config.php aan te passen)
    // define sql query string with special placeholders in the form of ?
    $sql = "SELECT * FROM users 
      WHERE gebruikersnaam=?
      AND wachtwoord =?";
    // prepare statement based on sql query string
    $statement = $connect->prepare($sql);
    // bind first question mark with value from $_POST, first question mark will be replaced with that value
    $statement->bindParam(1, $_POST['gebruikersnaam']);
    // do the same for second question mark
    $statement->bindParam(2, sha1($_POST['wachtwoord']));
    // execute this prepared statement with binded values
    $statement->execute();
    // fetch row from the result in the form of associated array
    if(($row = $statement->fetch(PDO::FETCH_ASSOC))){
        $_SESSION['gebruiker'] = $row['voornaam'];
        header("location: dashboard.php");
    }
    // free statement memory
    $statement = null;
}

Note: Code is not tested.

Edit, adding explanation: When working with PDO you should be using it's way of dealing with queries and database. Using any of mysql_* functions is not an optimal way of doing this.

Aleksandar Janković
  • 813
  • 1
  • 10
  • 18
  • Please explain what you changed, we want to learn people something. – Wouter J Feb 27 '13 at 09:57
  • Can we all just calm down, there is an edit button you know :). I need time to write clear and well written response. – Aleksandar Janković Feb 27 '13 at 10:08
  • Function works for 50%. When i enter my correct details, it still redirects me to the login page. EDIT: noticed the sha1 was forgotten in the script before you editted. It works now! :) – Michael Koelewijn Feb 27 '13 at 10:08
  • @Michael Just keep in mind that I modified your example to make it easer for you to understand what is going on. But generally this kind of code is of poor quality. – Aleksandar Janković Feb 27 '13 at 10:25
1
  • First of all, determine which type of error handling you want. PDO defaults to PDO::ERRMODE_SILENT which means that you don't get any errors. I recommend you to use PDO::ERRMODE_EXCEPTION which means that you need to work with try { ... } catch() { ... } blocks around your code.

  • Secondly, when you are using PDO, you can't use mysql_* functions. So using mysql_real_escape_string is not correct. Furthermore, because you are using prepared statements you don't need any SQL injection protection at all. But you need to use param binding.

  • You also have some mysql_query around line 7...

  • PDO does not have a build in mysql_num_rows function. You should put a COUNT(*) statement in your query for that. See also this answer

Community
  • 1
  • 1
Wouter J
  • 40,437
  • 15
  • 101
  • 111
0
function checkLoginPDO(){
$connect = new PDO(host, username, password); // Database Connectie maken (De host, username & password zijn in de config.php aan te passen)
$sql = "SELECT * FROM users 
      WHERE gebruikersnaam=:gebruikersnaam
      AND wachtwoord = :wachtwoord"; 
$value = $connect->prepare($sql);
$value->bind(':gebruikersnaam',$_POST['gebruikersnaam']);
$value->bind(':wachtwoord',sha1($_POST['wachtwoord']));
$value->execute();
$data = $value->fetchAll();
if(count($data) > 0){
    $_SESSION['gebruiker'] = $data[0]['voornaam'];
    header("location: dashboard.php");
}

}

user1452962
  • 388
  • 3
  • 8
0

Login functionality now works wonders! Thanks guys! I've just completed my image-upload script which writes the filename to the database. It works, but is it safe from stuff like sql-injection?

I am currently working on my last studyproject where security is a big issue. If i am able to pull a common security CMS off, i'll get my degree as programmer :)

Here's the function:

function uploadImage() {
    $dir = $_SERVER['DOCUMENT_ROOT'] . 'pvb/upload/';
    $allowedExts = array("jpg", "jpeg", "gif", "png");
    $extension = end(explode(".", $_FILES["file"]["name"]));
    if ((($_FILES["file"]["type"] == "image/gif")
            || ($_FILES["file"]["type"] == "image/jpeg")
            || ($_FILES["file"]["type"] == "image/png")
            || ($_FILES["file"]["type"] == "image/pjpeg"))
            && ($_FILES["file"]["size"] < 2000000)
            && in_array($extension, $allowedExts)) {
        if ($_FILES["file"]["error"] > 0) {
            echo "Return Code: " . $_FILES["file"]["error"] . "<br>";
        } else {
            if (file_exists($dir . $_FILES["file"]["name"])) {
                echo $_FILES["file"]["name"] . " already exists. ";
            } else {
                move_uploaded_file($_FILES["file"]["tmp_name"], $dir . $_FILES["file"]["name"]);
                $this->createThumbs($dir, $dir . "thumbs/", 100);

                $connect = new PDO(host, username, password); // Database Connectie maken (De host, username & password zijn in de config.php aan te passen)
                $sql = "INSERT INTO afbeeldingen (img_naam) VALUES (:naam)";
                $value = $connect->prepare($sql);
                $value->bindValue(":naam", $_FILES['file']['name'], PDO::PARAM_STR);
                $value->execute();
                $connect = null;
            }
        }
    } else {
        echo "Invalid file";
    }
}