Unsubscribe script not deleting database entry for user

UAFWolfcry · December 13, 2019

I'm creating a newsletter and the unsubscribe isn't deleting the database entry like I'm asking it to. Everything else works fine, it even successfully says the user has been removed, but it doesn't actually delete the database entry.

I've spent two days trying to figure out why. Here's the code:

Newsletter sign up:

<?php
//DB Connect Info
$servername = "";
$database = "";
$username = "";
$password = "";

// Create connection
$conn = mysqli_connect($servername, $username, $password, $database);

// Check connection
if ($conn->connect_error) {
die("Connection failed: " . $conn->connect_error);
}

/*$createTable = $conn->prepare ("CREATE TABLE IF NOT EXISTS email_user (
  id int(11) NOT NULL AUTO_INCREMENT,
  email varchar(200) NOT NULL,
  hash varchar(250) NOT NULL,
  PRIMARY KEY (id)
)");

$createTable->execute();
*/

function input_security($data)
{
    $data = trim($data);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    return $data;
}

$email = input_security($_POST['email']);

$insertData = input_security($insertData);

if(isset($_POST['submit']))
{  
  extract($_POST);
  if($email!="") :
    $email=mysqli_real_escape_string($conn,$email);
    $emailval = '/^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,4})$/';    
    if(preg_match($emailval, $email)) :
      $db_check=$conn->query("SELECT * FROM email_user WHERE email='$email'");
      $count=mysqli_num_rows($db_check);
      if($count< 1) :         
         $hash=md5($email.time());
         $link = '/unsubscribe.php?key='.$hash;

        // Change your domain        
         $fetch=$conn->query("INSERT INTO email_user(email,hash) VALUES('$email','$hash')");
         $to="$email"; //change to ur mail address
         $strSubject="Maintenance Fee Relief, LLC | Email Subscription";
         $message = '<p>Thank you for subscribing with us.</p>' ;              
         $message .= '<p>Click here to unsubscribe your email : <a href="'.$link.'">unsubscribe</p>' ;              
         $headers = 'MIME-Version: 1.0'."\r\n";
         $headers .= 'Content-type: text/html; charset=iso-8859-1'."\r\n";
         $headers .= "From: info@";            
         $mail_sent=mail($to, $strSubject, $message, $headers);  
         $msg_sucess="Your request has been accepted!.";
      else :
        $msg="This $email email address is already subscribe with us.";
      endif;  
    else :
      $msg="Please enter your valid email id";
    endif;      
  else :
    $msg="Please fill all mandatory fields";
  endif;
}
?>

<div class="newsletter-sign-up-header-form">
<div id="logerror"><?php echo @$msg; ?><?php echo @$msg_sucess; ?></div>
<form  method="post">
<span><input type="email" name="email" placeholder="Email Address - Join Newsletter" class="newsletter-sign-up-header-email" required></span>
<span><button name="submit" value="submit" title="Submit" class="newsletter-sign-up-header-submit-button">Submit</button></span>
</form>
</div>

<?php
//DB Connect Info
$servername = "";
$database = "";
$username = "";
$password = "";

// Create connection
$conn = mysqli_connect($servername, $username, $password, $database);

// Check connection
if ($conn->connect_error) {
die("Connection failed: " . $conn->connect_error);
}
?>

<?php
if(@$_GET['key']!=""):
    $hash=mysqli_real_escape_string($conn,$_GET['key']);
    $fetch=$conn->query("SELECT * FROM email_user WHERE hash = '$hash'");
    $count=mysqli_num_rows($fetch);
    if($count==1) :
      $row=mysqli_fetch_array($fetch);      
        $conn->query("DELETE email_user WHERE id='$user_id'");
        $msg="Your email id unsubscribe with us";
    else :
      $msg="Please click valid link.";
    endif;
else :
    header("Location:404.php");
endif;
?>

<!doctype html>
<html lang="en">
<head>
   <title>Unsubscribe</title>
</head>
<body>
    <div align="center">
    <h2><?php echo $msg; ?></h2>
        <a href="https://www.--.com">--.com</a>                                                
    </div>

Quote

gw1500se · December 13, 2019

This is really a MySQL question and belongs in that forum. However, I think your delete query is missing 'FROM <table name>'. You should be checking for errors after any query and handling them appropriately.

Barand · December 13, 2019

Put this line

mysqli_report(MYSQLI_REPORT_ERROR|MYSQLI_REPORT_STRICT);

before your mysqli connection line. Errors will then be reported automatically.

UAFWolfcry · December 13, 2019

Trying to follow this tutorial:

http://www.mostlikers.com/2015/11/simple-subscribe-and-unsubscribe-script.html

Psycho · December 13, 2019

As @gw1500se stated, the DELETE query is malformed as it does not include the "FROM" parameter, but it still won't work with that

$conn->query("DELETE email_user WHERE id='$user_id'");

$user_id is never defined!!! Aside from that, there are quite a few problems and poor construction. Here are some tips:

1. Put your DB connection logic in a separate file and include() it on the pages you need it. If you need identical code on multiple scripts, don't use copy/past coding. If you ever need to make a change in your DB connection you would only have to change it one place and not many.

2. Never user "SELECT *". There are performance and security problems with doing that. Always list the fields you want.

3. Don't suppress errors using the '@' symbol. You could have simply used an isset() check for the variable in your script.

if(isset($_GET['key']) && $_GET['key']!=""):

4. I would highly suggest not wrapping your error conditions in nested if/else conditions in this manner as it makes it difficult to easily "see" which results go with which error

if(!error_condition_1) {
    if(!error_condition_2) {
        if(!error_condition_3) {
            //Success result
        } else {
            //error result 3
        }
        //error result 2
    }
    //error result 1
}

This is MUCH more readable and maintainable IMO

if(error_condition_1) {
    //error result 1
} elseif(error_condition_2) {
    //error result 2
} elseif(error_condition_3) {
    //error result 3
} else {
    //Success result
}

5. You are switching back and forth between object oriented and procedural. Stick with one for consistency

    $fetch = $conn->query("SELECT * FROM email_user WHERE hash = '$hash'");
    //$count = mysqli_num_rows($fetch);
    $count = $fetch->num_rows;

6. Use prepared statements for your queries.

7. No need to use a SELECT statement before running the DELETE query. A race condition can create problematic results. You can just run the DELETE query and then check the number of affected rows. If 0, then there was no matching record, else if 1 then you can tell the user they were unsubscribed.

Barand · December 13, 2019

12 minutes ago, UAFWolfcry said:

Trying to follow this tutorial:

As you can see from @Psycho's comments, that was your first mistake.

UAFWolfcry · December 13, 2019

I'm in learning phase, I wanted to start with something simple. I had no idea it was poorly written. Thanks.

I'll go through your post slowly piece by piece and see if I can figure it out. If not I'll pop another question here.

Thanks for your help.

UAFWolfcry · December 13, 2019

I did realize the user_id variable was missing completely. I couldn't figure why they would omit that from their crappy tutorial and expect it to work.

What should the user_id variable look like so that I can delete the correct persons entry?

I just want to get it to work for the moment and then I can play around with the code, learn from it and make it better. I did realize they aren't using prepared statements and I was going to fix that after I got this stupid thing working.

Thanks.

Edited December 13, 2019 by UAFWolfcry

gw1500se · December 13, 2019

Depends on what the schema looks like. Something in on the page passed back when the delete is activated should uniquely identify the user to be deleted. Perhaps you should post the schema and explain what you want to accomplish.

UAFWolfcry · December 13, 2019

New at this, but here's the screenshot of the tables.

I have them fill out the form, it associates their entry with a hash, then sends them an email with an unsubscribe link they can click on. They click on unsubscribe and it's supposed to remove their entries under email_user.

mac_gyver · December 13, 2019

reproducing something you see on the web isn't learning. it's you functioning as a human photo-copier machine, where the quality of the output cannot be any better then the quality of the input, which in this case is filled with mistakes, bad and out of date practices.

ignoring for the moment that data is almost never actually deleted in real-life (its updated to indicate it is no longer being used), for the current operation of deleting a specific record in a database table, what inputs do you have or need, what processing are you going to do based on those inputs, and what result or output are you going to produce? defining the Inputs, Processing, and Output (IPO) is the most useful thing you can do before writing any code.

once you have defined these things, you can go about designing, writing, testing, and debugging the code needed to accomplish the steps it takes to perform this operation.

gw1500se · December 13, 2019

So the action is from an email? PHP is probably not the best solution for processing an incoming email, IMO. In any case the hash is probably what should be used for the delete. Something like:

DELETE FROM email_user WHERE hash='$hash'

Edited December 13, 2019 by gw1500se

UAFWolfcry · December 13, 2019

Thank you! That worked. Thank you so much.

Now that it works.. I am going to tear it into pieces and learn from it, make it better and continue to teach myself better PHP/SQL.

I realize using someone elses code isn't ideal, I'm a web developer and I've been writing my own HTML/CSS code for years. I started off by studying other peoples code, figuring out how it worked and why and then went on writing my own code. That's how my brain works.

UAFWolfcry · December 13, 2019

What language would you recommend for processing email?

mac_gyver · December 13, 2019

2 minutes ago, UAFWolfcry said:

What language would you recommend for processing email?

that information was a miss-statement. this code is not processing an email. it is processing the result of clicking on a link that was sent in an email.

another issue with this code is it ONLY works because you are reading the email on the same computer where the web server is running. the URL being sent in the email is a relative URL.

to do this properly, the link would need to be an absolute, fully qualified URL, so that when it is clicked/copied to a browser's address bar, it will work regardless of what computer the email is opened on.

UAFWolfcry · December 13, 2019

It's not a relative URL, I purposefully deleted the URL and DB info when I posted it here.

mac_gyver · December 13, 2019

your should x/* out information in posts, rather than to delete it, as that changes the meaning of what you post.

site, personal, sensitive, configuration information used in code should be defined in a separate .php configuration file and required into your main code. this will let you post any of your main code as is without needing to alter it.

UAFWolfcry · December 13, 2019

Thanks. Meaning to do that after I got it working. Was just piecing it together first. Will do that now.

Sign In

Unsubscribe script not deleting database entry for user

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information