0

I don't understand why this code is not working. I can upload a photo and delete photos from my database when they are saved as BLOB but I can not update as a BLOB? This is the code I am using but I can not see what I am doing wrong.

try {       
    if (file_exists($imageFile)){
        $photoToUpload = file_get_contents($imageFile);
        $stmt = $db->prepare("UPDATE db_birds SET photo = :photoToUpload WHERE ID = :userID AND userName = :userName");
        $stmt->execute(array(':photoToUpload' => $photoToUpload, ':ID' => $uid, 'userName' => $uname));
        if (file_exists($imageFile)) (unlink($imageFile));
    }
            
} catch( PDOException $e ) {
        print_r($e);
    }

This is the error I keep getting when trying to run the query.

PDOException Object ( [message:protected] => SQLSTATE[HY093]: Invalid parameter number: parameter was not defined [string:Exception:private] => [code:protected] => HY093 [file:protected] => C:\xampp\htdocs\admin\editadvert.php [line:protected] => 250 [trace:Exception:private] => Array ( [0] => Array ( [file] => C:\xampp\htdocs\admin\editadvert.php [line] => 250 [function] => execute [class] => PDOStatement [type] => -> [args] => Array ( [0] => Array ( [:photoToUpload] => ����JFIF``��>CREATOR: gd-jpeg v1.0 (using IJG JPEG v90), default quality

I have tried everything I can think of but to no avail so I thought I would ask for help.

EDIT: I found another solution which worked for me. This is my solution...

try {       
    if (file_exists($imageFile)){
        $photo = file_get_contents($imageFile);
        $query="UPDATE db_birds SET photo=:photo
        WHERE ID=:ID";
         
        $step=$db->prepare($query);
        $step->bindParam(':ID',$keys,PDO::PARAM_INT, 3);
        $step->bindParam(':photo',$photo,PDO::PARAM_LOB);
        $step->execute();
    }  
} catch( PDOException $e ) {
        //print_r($e);
        die();
    }
Charly Willy
  • 59
  • 7
  • When your code is vulnerable to SQL injection, it's probably easier to get random crashes with binary data. – Álvaro González Feb 02 '22 at 13:18
  • I don't see how it is vulnerable to SQL Injection? it looks ok to me – Charly Willy Feb 02 '22 at 13:20
  • 1
    As soon as the image raw data contains a byte that equals the ASCII single quote, bad things will happen. – Álvaro González Feb 02 '22 at 13:28
  • why does it allow us to add images, delete images but not update them? – Charly Willy Feb 02 '22 at 13:55
  • `I don't see how it is vulnerable to SQL Injection`...in that case, please read things like https://www.acunetix.com/websitesecurity/sql-injection/ (there are many others similar too you can read online) and then [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) to learn how to fix it. It will also prevent silly syntax errors caused by unescaped input values - e.g. a simple `'` in your input data would break the SQL syntax. – ADyson Feb 02 '22 at 14:04
  • on all input values I use htmlspecialchars or filter_input ect, nothing raw is allowed anywhere – Charly Willy Feb 02 '22 at 14:07
  • 1
    htmlspecialchars has nothing to do with SQL injection. The clue's in the name - it encodes HTML markup to prevent it being rendered as HTML in a browser. That should be used when you _output_ into a browser. It has no place in input filtering, and has nothing to do with SQL. The way you protect against SQL injection etc is to use prepared statements and parameters when building your SQL query. It's likely to also solve whatever problem you're having right now. – ADyson Feb 02 '22 at 14:10
  • 1
    Equally, the filters offered by filter_input can help to validate or sanitise the data in certain ways, but again none of them have _anything_ to do with SQL injection. You should familiarise yourself with what the filters actually do and what they help to protect against, rather than making any assumptions. The way to prevent SQL injection is clearly documented in many places online (again, for the record the method is this: you must use prepared statements and parameters when building your queries), and it is unrelated to both the functions you've just mentioned. – ADyson Feb 02 '22 at 14:24
  • The error is quite amusing. I never thought that neglecting prepared statements (or, rather, using a cargo cult prepared statement) would result in such an error. But here it is. – Your Common Sense Feb 02 '22 at 14:28
  • Thanks ADyson, I understand what you are saying now. I was looking for example PDO methods to achieve what it was I was trying to do, many of the topics I found on here were either voted down, too complex for me to understand or did not relate to my own issues. I thought I was using prepared statements to be fair – Charly Willy Feb 02 '22 at 14:47
  • You are using prepared statements it's true, but the other part of the recipe is parameters, which you aren't using. The protection doesn't work unless you do both – ADyson Feb 02 '22 at 15:12
  • I updated the code in the first post, with my haste I realized I posted the wrong block of code. It has been a tiresome night working on this. – Charly Willy Feb 02 '22 at 15:32
  • 1
    then you need a good night's sleep and then check the parameter names in the query (such as :photoToUpload) and the keys in the execute (such as :photoToUpload). check each pair whether they match exactly. – Your Common Sense Feb 02 '22 at 16:01
  • Hint: `:userID`...is in the SQL. Where does that occur in the array you pass to the execute() command? – ADyson Feb 02 '22 at 16:16
  • If I am honest I have been playing about with this now for around 14 hours, I think I will have a nap and hope to wake up with my brain in the right place. I need to think more about what I am doing wrong and why so that I can get it right. Your comments helped a lot but the (solved link) at the top did nothing for me. – Charly Willy Feb 02 '22 at 16:22
  • 1
    Honestly, the names just have to match in the sql and the parameter list...its not rocket science. `:userID` isn't the same as `:ID`. That was the point of the two hints you just received. So yeah if you didnt see it, you definitely need to take a break! :-) – ADyson Feb 02 '22 at 16:31
  • I actually have it working now, I found some old code on my external hard drive from last year. I am not sure how to use the mini markdown on here so not sure how to show the resulting code. it is nothing like the code I first posted at all. Maybe I should Update the top code again with the solution? – Charly Willy Feb 02 '22 at 16:39
  • 1
    Well the question is closed now, so that's why you can't post an answer. But yeah you could edit the question with the new code an extra bit if you really want (but don't destroy the current, problematic code as it defeats the point of the question). How different is it though? It looks like it's basically just a typo in your current code, as we've mentioned. – ADyson Feb 02 '22 at 16:43
  • I see. Well yes that's just different way of doing the parameter binding in PDO, but the effect is the same. And the constraint is the same in the sense that the parameter names in the SQL must match the ones you bind. But you've omitted the userName parameter entirely in this one - was it redundant, in fact? – ADyson Feb 02 '22 at 16:45
  • I updated the first post with the working code. I forgot to mention the variable $keys is just the ID number of where the photo is found in the db or row number. Thank god for saving old files :) appreciate your help though, it was something you wrote that made me remember I had something on an old drive. – Charly Willy Feb 02 '22 at 16:47
  • `I found some old code on my external hard drive from last year`...maybe time to implement a proper source control system then, so you've got a full history of every change you make, all in one place. And then back up the source control repo (it's easy if you publish to GitHub or one of those, or just backup the whole folder automatically to dropbox or similar) – ADyson Feb 02 '22 at 16:47
  • The $userName was just to make sure that not only was the correct row selected but also to make sure that it was belonging to the person who submitted the photo. I was planning on checking for username and user ID match to be double sure but I am happy with the results now – Charly Willy Feb 02 '22 at 16:50
  • 1
    this whole question is a mess and I see no value in it. Obviously, it suddenly started working only by **accident**, just the same way it didn't work. The "solution found" is as good as any other code suggested save it in't riddled with silly typo mistakes. – Your Common Sense Feb 02 '22 at 16:54
  • Your Common Sense, there is always one person isn't there? from the working code I can now see why my old code was not working so for me it is a learning curve but for you it seems an option to jeer at someone who was asking for help. – Charly Willy Feb 02 '22 at 16:57
  • The code you've shown first in the question would likely have worked too if you'd simply corrected the parameter name mismatch that we mentioned (and which is mentioned by the error message). Although of course we don't know if the extra parameter in the WHERE clause might have caused it to return less results - but it wouldn't have crashed, at least. – ADyson Feb 02 '22 at 16:59
  • When you said it was not fully a prepared statement it it me off using it. I had a browse about and looked for ways to make it a fully prepared statement. Now you mention it, I have a few other sections on my site I will need to correct as they work similar to the code I originally posted. I will have to fix that but that is for tomorrow :) I better go now anyway, I think my presence angers Sir 'Your Common Sense' – Charly Willy Feb 02 '22 at 17:03
  • 1
    `When you said it was not fully a prepared statement it it me off using it`...yeah but look at the timestamps - that was in relation to an [earlier version](https://stackoverflow.com/posts/70956318/revisions) which threw a different error, not the stuff you've got now. What you've got now at the start of your question is fully prepared and parameterised, but it just has the parameter names mismatched between the SQL and the parameter array. – ADyson Feb 02 '22 at 17:05
  • the other person on here was saying it was open to SQL injection, kept on criticizing the code, I realized afterwards I actually posted the wrong block of code. even after correcting the code it seemed still bad code. when you said it was part prepared and part not it made me realize that last year I was working on prepared statements for this topic. I just forgot I have those files until you reminded me – Charly Willy Feb 02 '22 at 17:10
  • `even after correcting the code it seemed still bad code`...no. I think maybe you read the comments at the wrong time or something. I posted that comment about it being part-prepared _before_ you changed the question to show the later version. – ADyson Feb 02 '22 at 17:12
  • 1
    `$db->prepare("UPDATE db_birds SET photo = :photoToUpload WHERE ID = :userID AND userName = :userName");` is fully prepared and parameterised, and contains no raw variable values, as you can clearly see for yourself. The only problem with it is that `:userID` in that string doesn't match the `:ID` name in `array(':photoToUpload' => $photoToUpload, ':ID' => $uid, 'userName' => $uname)` – ADyson Feb 02 '22 at 17:14
  • I realize now that ':ID' => $uid should have actually been ':ID' => $keys (the row number and not the ID of the person. ID is the row name (auto increment). After looking at the new (fixed code) I can see why the older code would not work. I think I have just been looking t it for too long – Charly Willy Feb 02 '22 at 17:21
  • 1
    Yes maybe. But also `WHERE ID = :userID` should be `WHERE ID = :ID` - that's what the error message is complaining about specifically. – ADyson Feb 02 '22 at 17:23

0 Answers0