0

I am trying to build a email verification. Sending an email with a link to the user is working. Now I want to set active = 1 when the user clicks on the link wich he received. I have checked the variables $email and $key they are getting the right information from the url. When the active is set to 1 I want to echo an ahref to login.php. I think there is someting wrong in my SQL query can somebody help?

<?php

if (isset($_GET['email'])) {
 $email = $_GET['email'];
}
if (isset($_GET['hash'])){
 $key = $_GET['hash'];
}

 $query = $mysqli->query("UPDATE `users` SET active=1 WHERE `email` = '". $email ."' AND `mailcheck` ='". $key ."' ");

 $result  = $query->fetch_row();

if($result == 1){

     echo "Your account is now active. You may now <a href="login.php">Log in</a>";

}
 else {
 echo "Your account could not be activated. Please recheck the link or contact the system administrator. test";
}


} 
?>
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
user3356007
  • 393
  • 1
  • 6
  • 20
  • 2
    Add error reporting to the top of your file(s) right after your opening PHP tag for example ` – Funk Forty Niner Jan 04 '16 at 16:42
  • Verify in the database if the field active is indeed being changed. – Phiter Jan 04 '16 at 16:44
  • Just a side note: There is definitely something *very* wrong in your SQL query, as you neither escape the `$email` and `$key` contents explicitly, nor use prepared statements to get this done for you automatically, — thus provoking SQL injections. – Anton Samsonov Jan 04 '16 at 16:45
  • One thing that could be happening is, if your column holding the hash isn't long enough, then MySQL will fail silently because of it. Your hash key is unknown as well as other variables. do a `var_dump();` and your other file may also be playing a role here. – Funk Forty Niner Jan 04 '16 at 16:46
  • I am working within bluemix so it wont display any errors @Fred-ii- – user3356007 Jan 04 '16 at 16:46
  • well I won't be able to help you there. If there are any errors somewhere, check your logs. – Funk Forty Niner Jan 04 '16 at 16:47
  • 1
    If it was me i would check first to see if a user in the database even exists with the given email / key ( both ) before going any further – Adam Jan 04 '16 at 16:52
  • @Adam - Agreed ^ and makes more sense really and I anticipated for it also. – Funk Forty Niner Jan 04 '16 at 17:09

2 Answers2

2

Hold on here. fetch_row() http://php.net/manual/en/mysqli-result.fetch-row.php is for a SELECT and not UPDATE.

What you're looking to use is mysqli_affected_rows()

on UPDATE in order to check if the update was successful.

If you're looking to do a SELECT here (which makes more sense really), then you need to use mysqli_num_rows(), and if both exists, then do the UPDATE.

You should also check for errors against your query:


If a row/user exists:

Consult an answer of mine https://stackoverflow.com/a/22253579/1415724 to check if a user exists, where you can base yourself on it.


Plus, a suggestion. Use !empty() instead of isset(). It's usually best to check against values.

What would also work better is to check if any are empty, rather than 2 conditional statements.

If one is left empty, your code will continue to execute and in turn, your query failing.

If you want to keep your present method, then you should exit; after each GET, but I wouldn't recommend it.

More like:

if ( !empty($_GET['email']) && !empty($_GET['hash']) ) {
  $email = $_GET['email'];
  $key = $_GET['hash'];
}

else{ exit; }

Your present code is open to SQL injection. Use mysqli_* with prepared statements, or PDO with prepared statements.

Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • @user3356007 ok. Plus, I did make a few more edits to my answer if you would like to reload it, in case you may not have seen them. – Funk Forty Niner Jan 04 '16 at 17:15
  • @user3356007 can you define "not work"? any errors and can you update your question to show us the code you are now using, but DO NOT overwrite your original question/code, but to mark it as an edit http://stackoverflow.com/posts/34595724/edit under it and marked as *"Here is my new/updated code..."*. – Funk Forty Niner Jan 04 '16 at 18:35
  • it is updating in the database now I think something is going wrong in the IF statement after updating – user3356007 Jan 04 '16 at 18:52
  • @user3356007 you went and did the complete contrary of what I said. You overwrote your original post completely and people visiting the question and my answer, will see it and I stand at being downvoted for it. My answer to you fixed the original problem and is updating your db, therefore you need to set your question to its original state and mark my answer as being correct. Edit: I have performed a rollback to its original state. – Funk Forty Niner Jan 04 '16 at 18:54
  • @user3356007 so now you decided to unaccept my answer. why is that? – Funk Forty Niner Jan 04 '16 at 19:27
  • Thanks @fred-ii- your answer was correct i needed the affected_rows don`t know what went wrong! – user3356007 Jan 04 '16 at 19:37
  • @user3356007 you're welcome. However I can't understand why you decided to accept the other answer instead, while I was right all along and by the timestamp, my answer was first. But you can choose whichever you want, that's up to you. I'm glad the matter was resolved, but I still don't understand why you keep going in and out like that. – Funk Forty Niner Jan 04 '16 at 19:39
  • I wanted to like both of you because the code he upload was also right did not know i could net accept both answers so your where first I gave you the green V – user3356007 Jan 04 '16 at 19:41
  • @user3356007 yeah, that would be good if Stack would let us do that. Unfortunately that is not the case but I have upvoted his answer to compensate. – Funk Forty Niner Jan 04 '16 at 19:42
  • @Fred-ii- I personally like your answer since it contains more explanation than mine, and it will be helpful for the future visitors of SO. One more upvote from my side. :) – Rajdeep Paul Jan 04 '16 at 19:43
  • @RajdeepPaul Thank you Rajdeep, we're all in this for the "good". *Cheers* – Funk Forty Niner Jan 04 '16 at 19:44
2

The problem is because of the following line,

$result  = $query->fetch_row();

You're trying to do UPDATE operation but you're actually fetching the result row using ->fetch_row() statement, which by the way doesn't exist because UPDATE operation doesn't return any result set.

Use ->affected_rows property to get the number of affected rows from the UPDATE operation, like this:

$mysqli->query("UPDATE `users` SET active=1 WHERE `email` = '". $email ."' AND `mailcheck` ='". $key ."'");

if($mysqli->affected_rows ==  1){
    echo "Your account is now active. You may now <a href=\"login.php\">Log in</a>";
}else{
    echo "Your account could not be activated. Please recheck the link or contact the system administrator.";
}

Here's the reference:

Edited:

Your code on the validation page should be like this:

if(isset($_GET['email']) && isset($_GET['hash'])){
    $email = htmlentities($_GET['email']);
    $key = htmlentities($_GET['hash']);

    $mysqli->query("UPDATE `users` SET active=1 WHERE `email` = '". $email ."' AND `mailcheck` ='". $key ."'");

    if($mysqli->affected_rows){
        echo "Your account is now active. You may now <a href=\"login.php\">Log in</a>";
    }else{
        echo "Your account could not be activated. Please recheck the link or contact the system administrator.";
    }
}else{
    echo "wrong parameters.";
}

Re-edited:

After extensive debugging with OP the issue is resolved now, and this is the final working code,

if (isset($_GET['email']) && isset($_GET['hash'])) { 
    $email = $_GET['email']; 
    $key = $_GET['hash']; 

    $mysqli->query("UPDATE `users` SET active=1 WHERE `email` = '". $email ."' AND `mailcheck` ='". $key ."' "); 


    if($mysqli->affected_rows) { 

        echo "Your account is now active"; 

    }else { 
        echo "Failed"; 
    } 
}
Rajdeep Paul
  • 16,887
  • 3
  • 18
  • 37
  • tried your solution but it did not work @Rajdeep paul – user3356007 Jan 04 '16 at 17:03
  • @user3356007 I don't see any reason why it shouldn't work. Make sure that you're getting `$email` and `$key` correctly. Also make sure that the `active` column is not already set to `1`. – Rajdeep Paul Jan 04 '16 at 17:47
  • @user3356007 I've updated my answer. Please see the **edited** section of my answer. – Rajdeep Paul Jan 04 '16 at 17:58
  • @user3356007 From your updated question, this statement `$result = $query->mysqli_affected_rows();` is wrong. It should be `$result = $mysqli->affected_rows;` Edit: Never mind, since I can see a green tick I think your issue is resolved now. – Rajdeep Paul Jan 04 '16 at 18:56
  • thanks for your help @RajdeepPaul going to try this tomorrow I am very tired now! – user3356007 Jan 04 '16 at 19:06
  • very strange the code is working but i am not getting any echo @rajdeep paul – user3356007 Jan 04 '16 at 19:17
  • @user3356007 May be the `UPDATE` query updates more than one row. Are you getting this message, `Your account could not be activated. Please recheck...`, or is it complete blank? – Rajdeep Paul Jan 04 '16 at 19:21
  • It is complete blank but when i look in the PHPMYADMIN is has set active to 1 – user3356007 Jan 04 '16 at 19:23
  • @user3356007 That's because there's a syntax error in your code. Your `echo` statement should be like this, `echo "Your account is now active. You may now Log in";`. Escape the inner double quotes with backslash. I had suggested this thing in my answer also. – Rajdeep Paul Jan 04 '16 at 19:24
  • I know i have deleted that part just put in echo "succes"; but still doesn`t ego anything @Rajdeep Paul do you have skpye? – user3356007 Jan 04 '16 at 19:27
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/99724/discussion-between-rajdeep-paul-and-user3356007). – Rajdeep Paul Jan 04 '16 at 19:27