There is a mistake somewhere....

mrfdes · May 17, 2014

But where???

I have set up a script for people to remove themselves from a mailing list.

It worked fine so far, but when I added a possibility to show a message the entered email addres did not exist, things went wrong.

The script still runs, there are no errors, but when the user enters a non-existing email address, it still says "You have been removed".

Everything seemed to go well until I added a button to a URL (echo'd) rather than a plain link.

I have checked and rechecked, re-examined the flow, but I cannot find where I went wrong.

One further thing: the people get sent to this page by a one field form on a HTML page, with method "post".

Anyway, here is the script:

<HTML>
<HEAD>
 <TITLE>Vlaanderen-Flanders</TITLE>
</HEAD>
<BODY>

<?php
 
// Always try to connect and select the DB before anything else
$con = mysql_connect("localhost","jingleko_reload","*******") or die("Couldnt Connect to DB - ".mysql_error());
mysql_select_db("jingleko_reloader", $con) or die("Couldnt Select a DB - ".mysql_error());

// Set post var
$Epost = trim(addslashes(strip_tags($_POST['Epost'])));

// Look for it in DB
$query = "SELECT Epost FROM newsletter WHERE Epost='".$Epost."'";
$result = mysql_query($query);
//If found, do next thing
if(isset($_POST['Epost']))
{
    mysql_query("DELETE FROM newsletter WHERE Epost='$Epost'") or die (mysql_error());
    echo "<div align=\"center\"><img src=\"Pics/Vlaamse Leeuw.jpg\" width=\"114\" height=\"127\" border=\"0\"></div>";
    echo "<p align=\"center\"><b>Thank you, you are now removed from the list.</b></p><br>";
    echo "<p align=\"center\"><a href=\"index.htm\"><img src=\"Pics/begin.gif\" width=\"95\" height=\"30\" border=\"0\"></a></p>";
}
else
{
echo "<div align=\"center\"><b><font color=\"red\">This address does not exist</font></b></div><br>";
echo "<div align=\"center\"><a href=\"eruit.htm\"><img src=\"Pics/herbegin.gif\" width=\"95\" height=\"30\" border=\"0\"></a>"; 
echo "<a href=\"index.htm\"><img src=\"Pics/begin.gif\" width=\"95\" height=\"30\" border=\"0\"></a></div>";
}

mysql_close($con);

?>

</BODY>
</HTML>

Any ideas, please?

Sometimes when I altered the code, I just got a blank page.

Thanks in advance.

adam_bray · May 17, 2014

This if statement isn't doing what you want it to -

//If found, do next thing
if(isset($_POST['Epost']))

If you're going to use that then place the select query within it as well.

You want to replace that line (the one wrapping the delete query) with a count of the select results, checking the number of rows from the query doesn't equal 0.

Also, use PDO or mysqli_ instead of mysql_, those functions are depreciated.

PravinS · May 17, 2014

your php code should be like this

<?php
    // Always try to connect and select the DB before anything else
    $con = mysql_connect("localhost", "jingleko_reload", "*******") or die("Couldnt Connect to DB - ".mysql_error());
    mysql_select_db("jingleko_reloader", $con) or die("Couldnt Select a DB - ".mysql_error());

    // Set post var
    $Epost = trim(addslashes(strip_tags($_POST['Epost'])));
    if (isset($_POST['Epost']))
    {
        // Look for it in DB
        $query = "SELECT Epost FROM newsletter WHERE Epost='".$Epost."'";
        $result = mysql_query($query);
        //If found, do next thing
        if (mysql_num_rows($result) > 0)
        {
            mysql_query("DELETE FROM newsletter WHERE Epost='$Epost'") or die(mysql_error());
            echo "<div align=\"center\"><img src=\"Pics/Vlaamse Leeuw.jpg\" width=\"114\" height=\"127\" border=\"0\"></div>";
            echo "<p align=\"center\"><b>Thank you, you are now removed from the list.</b></p><br>";
            echo "<p align=\"center\"><a href=\"index.htm\"><img src=\"Pics/begin.gif\" width=\"95\" height=\"30\" border=\"0\"></a></p>";
        }
        else
        {
            echo "<div align=\"center\"><b><font color=\"red\">This address does not exist</font></b></div><br>";
            echo "<div align=\"center\"><a href=\"eruit.htm\"><img src=\"Pics/herbegin.gif\" width=\"95\" height=\"30\" border=\"0\"></a>";
            echo "<a href=\"index.htm\"><img src=\"Pics/begin.gif\" width=\"95\" height=\"30\" border=\"0\"></a></div>";
        }        
    }
    mysql_close($con);
?>

mrfdes · May 17, 2014

WOW!

That did the trick.

I will now try to work out why the count (or absence thereof) messed up the whole script.

Thank you so much gentlemen.

More stuff to study then.

Ch0cu3r · May 17, 2014

Id advise not to use addslashes for escaping user input. Instead use mysql_real_escape_string

Or preferably change your code over to PDO or MySQLi and use prepared statements. Please note that the mysql_* functions are deprecated which means they are no longer supported and could removed from future versions of PHP.

mrfdes · May 17, 2014

Thanks.

I have changed to mysqli now.

PDO is still a bit beyond me, I'm afraid, but I am working on that.

By the way, it looks like some mysqli statements are no longer supported when you change from mysql to mysqli.

But, thank you all for your very useful advice.

Ch0cu3r · May 17, 2014

By the way, it looks like some mysqli statements are no longer supported when you change from mysql to mysqli.

What do you mean that?

mrfdes · May 17, 2014

What I mean is:

when I changed

mysql_select_db("jingleko_reloader", $con) or die("Couldnt Select a DB - ".mysql_error());

to

mysqli_select_db("jingleko_reloader", $con) or die("Couldnt Select a DB - ".mysql_error());

I got the error 'Could not connect to db', so I had to put the database name in the mysqli_connect as 4th parameter.

Strange, but it is probably me.

Thank you.

Jacques1 · May 17, 2014

If you find MySQLi easier to learn than PDO, then clearly you're using it the wrong way. You've obviously just added an “i” to each function call in the hopes that this will somehow magically convert everything to MySQLi.

It doesn't work like this. If you want to update your code (which is a good idea!), you need to actually rewrite it and get rid of bad habits. For example, values are no longer inserted directly into the query string. This is extremely insecure and has lead to countless of SQL injection vulnerabilities. Instead, you use parameterized statements to securely pass data to queries.

Unfortunately, that's when MySQLi turns out to be very complicated and very cumbersome. Take a trivial task like fetching all forum posts from one member in a particular category:

<?php

/*
 * Make MySQLi throw an exception in case of an error. Without this, you have to
 * manually check every single return value to find out if there was a problem.
 */
$mysqli_driver = new mysqli_driver();
$mysqli_driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT;

$database = new mysqli('localhost', 'someuser', 'somepassword', 'somedatabase');
$database->set_charset('utf8');

// Use a parameterized statement to securely pass the data to the query.
$forum_posts_stmt = $database->prepare('
    SELECT
        forum_post_id,
        content
    FROM
        forum_posts
    WHERE
        author = ?
        AND category = ?
');
$forum_posts_stmt->bind_param('ii', $_GET['author'], $_GET['category']);
$forum_posts_stmt->execute();
$forum_posts_stmt->bind_result($forum_post_id, $content);

header('Content-Type: text/html;charset=utf-8');

while ($forum_posts_stmt->fetch())
{
    echo '<p>' . htmlspecialchars('Post ' . $forum_post_id . ' says: ' . $content) . '</p>';
}

Don't tell me this is easy. The statement alone requires five different methods.

PDO is much more straightfoward. You only need prepare(), execute() and a plain foreach loop:

<?php

$database_options = array(
    PDO::ATTR_EMULATE_PREPARES => false,
    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
);
$database = new PDO('mysql:host=localhost;dbname=somedatabase;charset=utf8', 'someuser', 'somepassword');

$forum_posts_stmt = $database->prepare('
    SELECT
        forum_post_id,
        content
    FROM
        forum_posts
    WHERE
        author = :author
        AND category = :category
');
$forum_posts_stmt->execute(array(
   'author' => $_GET['author'],
    'category' => $_GET['category'],
));

header('Content-Type: text/html;charset=utf-8');

foreach ($forum_posts_stmt as $forum_post)
{
    echo '<p>' . htmlspecialchars('Post ' . $forum_post['forum_post_id'] . ' says: ' . $forum_post['content']) . '</p>';
}

In addition to that, PDO isn't limited to MySQL. It's a universal interface for all mainstream SQL database systems.

So why use MySQLi? Just because the name sounds familiar?

mrfdes · May 17, 2014

Thank you for that Jacques.

But, "So why use MySQLi?", well, because I don't know any better.

The course I am following at the moment doesn't even mention mysqli, let alone PDO.

Like I said, I am a beginner, and a lot of things still look/sound extremely complicated tome.

Thank you for your input anyway.

ignace · May 17, 2014

That is because the course you are following is outdated.

A good place to start is this:

http://www.phptherightway.com/

mrfdes · May 18, 2014

Thank you, Ignace.

I am reading through it, but, to be quite honest, it does not look like a PHP tutorial to me.

Some bits might as well have been written in Chinese.

It gives some short examples, but it looks very complicated to me.

Thank you.

Jacques1 · May 18, 2014

If you just want to learn PDO, this wiki should help you.

Since the old MySQL extension is based on 90s technology, you will have to learn some new concepts like object-oriented programming, the already mentioned parameterized statements or proper error handling with exceptions. But don't worry, it's no rocket science.

If you read this very short tutorial and then simply play with PDO a bit, you should understand it very quickly. In fact, PDO is much more intuitive than the old extension. It's just that people have gotten used to copying and pasting the same old code, so anything new looks scary at first.

ignace · May 18, 2014

Some bits might as well have been written in Chinese.

It has:

http://wulijun.github.io/php-the-right-way/

ignace · May 18, 2014

Thank you, Ignace.

I am reading through it, but, to be quite honest, it does not look like a PHP tutorial to me.

Some bits might as well have been written in Chinese.

It gives some short examples, but it looks very complicated to me.

Thank you.

All jokes aside. What I was referring to is:

http://www.phptherightway.com/#databases_abstraction_layers

More specifically:

https://github.com/auraphp/Aura.Sql

It provides you with a simplified interface (Facade) to work with PDO, hiding it's 'complex' parts which might be a good starting point.

Dutch:

Van waar uit vlaanderen ben je? ik ben van vlaams-brabant.

Edited May 18, 2014 by ignace

mrfdes · May 18, 2014

All jokes aside. What I was referring to is:

http://www.phptherightway.com/#databases_abstraction_layers

More specifically:

https://github.com/auraphp/Aura.Sql

It provides you with a simplified interface (Facade) to work with PDO, hiding it's 'complex' parts which might be a good starting point.

Dutch:

Van waar uit vlaanderen ben je? ik ben van vlaams-brabant.

Van Oostende, maar ik woon al 20 jaar in Engeland.

mogosselin · May 19, 2014

@mrfdes

http://www.phptherightway.com/ isn't a tutorial, it's more of stuff that you should be careful with or know when using PHP.

It's just like if you started to paint a house. A tutorial would be "How to paint a wall with a brush".

And "painting the right way" would be more like : Use a brush #34 with Xyz type of paint. Be careful not to use the ABC outdated paint. You could also use a paint roller. etc.

You'll still need to do some reasearch and read tutorials, but at least you know the pitfalls and what to be careful with

Sign In

There is a mistake somewhere....

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

Join the conversation

Important Information