PDO/mysql UPDATE query doesn't do anything or display errors

Adam Copley

The code that starts on line 49 is doing absolutely nothing. I have tried to display PHP errors, used try and catch with the PDO set attributes etc, which also didn't display an error.

The code worked before in mysqli when I was using the mysql extension to connect but I'm currently in the process of converting the entire application to PDO.

    <?php 
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
//mysqli_report(MYSQLI_REPORT_ALL);
error_reporting(E_ALL);
ini_set("display_errors", 1);

if(!isset($_SESSION['eid'])){ header("Location: index.php"); } else {

require('dbconn.php');
$sessionuser = $_SESSION['eid'];
$messageid = $_GET['id'];

try{
$db = new PDO($dsn, $db_user, $db_pass);
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

$sql = "SELECT * FROM messages WHERE id = :messageid"; 
$rs_result1 = $db->prepare($sql);
$rs_result1->bindParam(":messageid", $messageid);
$rs_result1->execute();
$result1 = $rs_result1->fetch(PDO::FETCH_ASSOC);
$senderid = $result1['eidfrom'];
$recid = $result1['eidto'];

$sql1 = "SELECT * FROM employees WHERE eid = :senderid";
$rs_result2 = $db->prepare($sql1);
$rs_result2->bindParam(":senderid", $senderid);
$rs_result2->execute();
$result2 = $rs_result2->fetch(PDO::FETCH_ASSOC);

$sql2 = "SELECT * FROM employees WHERE eid = :recid";
$rs_result3 = $db->prepare($sql2);
$rs_result3->bindParam(":recid", $recid);
$rs_result3->execute();
$result3 = $rs_result3->fetch(PDO::FETCH_ASSOC);


          echo "<table>";
          echo "<tr><td>To: </td> <td>".$result3['fname']." ".$result3['lname']."</td></tr>";
          echo "<tr><td>From: </td> <td>". $result2['fname'] ." ".$result2['lname']."</td></tr>";
          echo "<tr><td>Date: </td><td> ". date("l, jS F Y H:i:s", strtotime($result1['date']))."<br /> </td></tr>";
          echo "<tr><td>Subject: </td><td>".$result1['subject']."</td></tr>";
          echo "<tr><td colspan='2'><img src =\"images/newssplit.gif\"></td></tr>";
          echo "<tr><td>Message: </td><td>". $result1['body']." </td></tr>";
          echo "</table>";

          //line 49 below
          if($sessionuser == $senderid) { 
            $sql3 = "UPDATE `messages` SET `reads`='1' WHERE `id`= :messageid";
            $result4 = $db->prepare($sql3);
            $result4->bindParam(":messageid", $messageid);
            $result4->execute();

             } else {
                    $sql4 = "UPDATE `messages` SET `read`='1' WHERE `id`= :messageid";
                    $result5 = $db->prepare($sql4);
                    $result5->bindParam(":messageid", $messageid);
                    $result5->execute();
                    }

} catch (mysqli_sql_exception $e) { 
        throw $e; 
    }
}
?>

To say the least I am stuck! I've read many a post on here with people having the same issues, and I don't see anything wrong with the code. What am I missing?

EDIT: So far I have checked the schema to ensure that my fields to actually exist, tried using query(), tried using standard variables rather than bindParam placeholders, The variable $messageid definitely has a value at that stage, as I test printed $sql3 after replacing :messageid with $messageid. I have posted some related files and the export of the schema in a zip ZIP. Haven't come to a solution yet, very stuck on this, as the UPDATE query on line 42 of inbox.php works just fine.

EDIT2: Code above updated with safer select queries, schema has been updated with correct data types and indexes cleaned up. But still what's now on Line 49 will not update the value in messages, OR return an error.

EDIT::SOLVED:

The problem wasn't my query, but my if statement. I hadn't fully tested the functionality of the statement and the queries. What I was doing was testing the queries on a message to and from the same user. An eventuality which I hadn't prepared my if statement for (as it happens the statement and queries combined were working all along for normal user 1 to user 2 and vice versa messages). Here's how I got it to work.

if($sessionuser == $senderid && $sessionuser == $recid) { 

        $result4 = $db->prepare("UPDATE `messages` SET `read_s`='1', `read_`='1' WHERE `id`= :messageid");
        $result4->bindParam(":messageid", $messageid);
        $result4->execute();

         } elseif($sessionuser == $senderid) {

                $result5 = $db->prepare("UPDATE `messages` SET `read_s`='1' WHERE `id`= :messageid");
                $result5->bindParam(":messageid", $messageid);
                $result5->execute();
                } else {

                $result6 = $db->prepare("UPDATE `messages` SET `read_`='1' WHERE `id`= :messageid");
                $result6->bindParam(":messageid", $messageid);
                $result6->execute();
                }

I changed the column headers from reads and read, to underscored after reading something about reserved words. But then also found it that it actually didn't matter. Thanks for the help everyone!!! The other notes and feedback that I got regarding the schema etc have helped me learn some good practice!! TYTY

Drew

based on your provided zip file in comments

Schema

CREATE TABLE IF NOT EXISTS `employees` (
  `eid` int(11) NOT NULL AUTO_INCREMENT,
  `fname` varchar(50) NOT NULL,
  `lname` varchar(50) NOT NULL,
  `dob` varchar(50) NOT NULL,   -- why not date datatype?
  `sdate` varchar(50) NOT NULL, -- why not date datatype?
  `address1` text NOT NULL,
  `address2` text NOT NULL,
  `city` varchar(50) NOT NULL,
  `postcode` varchar(50) NOT NULL,
  `telephone` varchar(50) NOT NULL,
  `mobile` varchar(50) NOT NULL,
  `email` text NOT NULL,    -- why text?
  `password` varchar(50) NOT NULL,  -- I can help you solve this next
  `depid` int(11) NOT NULL,
  `userlevel` int(11) NOT NULL,
  `blocked` int(11) NOT NULL,
  PRIMARY KEY (`eid`),  -- makes sense
  UNIQUE KEY `eid` (`eid`)  -- why a duplicate key (as PK) ?  you already have it covered
) ENGINE=MyISAM;
truncate table employees;

insert employees(fname,lname,dob,sdate,address1,address2,city,postcode,telephone,mobile,email,password,depid,userlevel,blocked) values
('Frank','Smith','dob','sdate','addr1','addr2','c','p','t','m','e','p',1,2,0);
insert employees(fname,lname,dob,sdate,address1,address2,city,postcode,telephone,mobile,email,password,depid,userlevel,blocked) values
('Sally','Jacobs','dob','sdate','addr1','addr2','c','p','t','m','e','p',1,2,0);



CREATE TABLE IF NOT EXISTS `messages` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `eidto` int(11) NOT NULL,
  `eidfrom` int(11) NOT NULL,
  `read` int(11) NOT NULL,
  `reads` int(11) NOT NULL,
  `inbox` int(11) NOT NULL,
  `sentbox` int(11) NOT NULL,
  `subject` text NOT NULL,  -- why a text datatype? was it gonna be huge?
  `body` text NOT NULL,
  `date` varchar(50) NOT NULL,  -- why this data type?
  PRIMARY KEY (`id`),       -- makes sense
  UNIQUE KEY `id` (`id`),   -- why this dupe?
  KEY `id_2` (`id`)         -- why?
) ENGINE=MyISAM;

insert messages(eidto,eidfrom,`read`,`reads`,inbox,sentbox,subject,body,`date`) values
(1,2,1,1,1,1,'subject','body','thedatething');

inbox.php

<?php 
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
//mysqli_report(MYSQLI_REPORT_ALL);
error_reporting(E_ALL);
ini_set("display_errors", 1);

session_start();
//$sessionuser = $_SESSION['eid'];
$sessionuser = 1;
require('dbconn.php');

    try {
        $db = new PDO($dsn, $db_user, $db_pass);
        $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);   // line added
        $db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);       // line added

        if (isset($_GET["page"])) { 
            $page  = $_GET["page"]; 
        } 
        else{ 
            $page=1; 
        }; 

        $start_from = ($page-1) * 10; 
        $sql = "SELECT * FROM messages WHERE eidto = $sessionuser AND inbox = 1 ORDER BY date DESC LIMIT $start_from, 10"; 
        echo $sql;
        $sql2 = "SELECT COUNT(*) FROM messages WHERE eidto = $sessionuser AND inbox = 1 ORDER BY date DESC LIMIT $start_from, 10"; 
        $rs_result = $db->query($sql);
        $count = $db->query($sql2)->fetchColumn();

        echo "<h3>Inbox</h3>";
        echo "<form action='".$PHP_SELF."' method='post'><table><tr><b><td>#</td><td>From</td><td>Subject</td></b></tr>";
        while ($row = $rs_result->fetch(PDO::FETCH_ASSOC)) { 
            $senderid = $row['eidfrom'];
            echo "<br>".$senderid;
            $messageid = $row['id'];
            $result2 = $db->query("SELECT * FROM employees WHERE eid = $senderid");
            $row2 = $result2->fetch(PDO::FETCH_ASSOC);
            if($row['read'] == 0) {
                echo "<tr> <td><input type='checkbox' name='checkbox[]' value='".$row['id']."' id='checkbox[]'></td>";
                echo "<td><b>".$row2['fname']." ".$row2['lname']."</b></td>";
                echo "<td><b><u><a href='usercp.php?action=messages&f=message&id=".$row['id']."'>".$row['subject']."</a></u></b></td></tr>";
            } else {
                echo "<tr> <td><input type='checkbox' name='checkbox[]' value='".$row['id']."' id='checkbox[]'></td>";
                echo "<td>".$row2['fname']." ".$row2['lname']."</td>";
                echo "<td><a href='usercp.php?action=messages&f=message&id=".$row['id']."'>".$row['subject']."</a></td></tr>";
            }
        }; 
        echo "<tr><td><input type='submit' id='delete' name='delete' value='Delete'></table></form>";

        if(isset($_POST['delete'])){
            for($i=0;$i<$count;$i++){
                $del_id = $_POST['checkbox'][$i];
                $sql = "UPDATE `messages` SET `inbox`='0' WHERE `id`='$del_id'";
                $result = $db->prepare($sql);
                $result->execute();
            }

            if($result){
                echo "<meta http-equiv=\"refresh\" content=\"0;URL=usercp.php?action=messages&f=inbox\">";
            }
        } 

        // NEED TO MODIFY CODE BELOW SO THAT PREVIOUS LINK DOESN'T LINK TO PAGE 0 WHEN ON PAGE 1
        // AND NEXT DISAPPEARS WHEN ON LAST PAGE WITH RECORDS
        $sql = "SELECT * FROM messages WHERE eidto = $sessionuser AND inbox = 1"; 
        $sql2 = "SELECT COUNT(*) FROM messages WHERE eidto = $sessionuser AND inbox = 1";
        $rs_result = $db->query($sql);
        $rows = $db->query($sql2)->fetchColumn();

        if($rows > 10) {
            $total_pages = ceil($rows / 10); 
            echo "<a href='usercp.php?action=messages&f=inbox&page=".($page-1)."'>Previous</a>";
            for ($i=1; $i<=$total_pages; $i++) { 
                        echo "<a href='usercp.php?action=messages&f=inbox&page=".$i."'>".$i."</a> "; 
            };  echo "<a href='usercp.php?action=messages&f=inbox&page=".($page+1)."'>Next</a>";
        } else { } 
    } catch (mysqli_sql_exception $e) { 
        throw $e; 
    }
?>

Check for errors. You had a typo on fetchColumn, something error reporting told me.

Add error reporting to the top of your file(s) which will help find errors.

<?php 
mysqli_report(MYSQLI_REPORT_ALL);
error_reporting(E_ALL);
ini_set('display_errors', 1);

The Op has said he is re-writing his code to PDO. As such,

  • You need to switch to PDO with prepared statements for parameter passing to protect against SQL Injection attacks.

By the way, above line and color poached from Fred's great PHP Answers

Note that I added the try/catch block, and a PDO connection exception attribute to support it.

Here is a checklist of things to cleanup:

  • Move your GETS toward $_POST due to url caching security concerns, and size limitations.

  • If you haven't, look into hashing and password_verify. Here is An Example I wrote.

  • Clean up the datatypes and indexes. Comments are seen in your schema.

  • Move to safe prepared statements as mentioned above.

So, as for functionality given here, the fake data I inserted appears, and the delete works. I am sure you can take it from here.

Edit

Best practice is to choose column names that are not Reserved Words. The ones in that list with an (R) next to them. The use of back-ticks around all column names in queries is a safeguard against query failure in that regard.

As far as your question of why I back-ticked some and not others. It was 3am, those were showing up as red in my query editor, and I was being lazy in not back-ticking all of them.

Collected from the Internet

Please contact [email protected] to delete if infringement.

edited at
0

Comments

0 comments
Login to comment

Related