0

I'm writting a HTML form for newsletter subscribers. So far the form ignores duplicates entries by making the subscriber_email field unique in the mysql table and using:

"INSERT IGNORE INTO newsletter_subscriber_popup (subscriber_name, subscriber_email) VALUES ('$subscriber_name', '$subscriber_email')";

I'm trying to improve the form so it updates the other fields when the email already exists in the table, and if the email does not exists then it inserts a new record. For that I'm doing this (so far it works but I feel it's not the proper way to do it):

    //HTML form variables----------------------------------------------------//
    $subscriber_name = mysql_real_escape_string($_POST['subscriber_name']);
    $subscriber_email = mysql_real_escape_string($_POST['subscriber_email']);    

    //Try update into DB---------------------------------------------------------//
    $sqlUpdate =
            "UPDATE newsletter_subscriber_popup 
            SET subscriber_name = '$subscriber_name'
            WHERE subscriber_email = '$subscriber_email'";

    if(!mysql_query($sqlUpdate)){
        //Insert into DB--------------------------------------------------------//
        $sqlInsert =
                "INSERT IGNORE INTO newsletter_subscriber_popup (subscriber_name, subscriber_email) 
                VALUES ('$subscriber_name', '$subscriber_email')";

        if(!mysql_query($sqlInsert)){
            die('Error: ' .mysql_error());
        }
    }

The scripts works when the subscriber_email exists and it updates the other fields but it fails when it should insert a new record.

EDIT----------------------------

ON DUPLICATED KEY UPDATE is what I was looking for. The script now updates the other fields when the subscriber_email already exists, and insert a new record when the the subscriber_email does not exists.

    //HTML form variables----------------------------------------------------//
    $subscriber_name = mysql_real_escape_string($_POST['subscriber_name']);
    $subscriber_email = mysql_real_escape_string($_POST['subscriber_email']);    

    //Insert into DB--------------------------------------------------------//
    $sqlName =
            "INSERT IGNORE INTO newsletter_subscriber_popup (subscriber_name, subscriber_email) 
            VALUES ('$subscriber_name', '$subscriber_email')
            ON DUPLICATE KEY UPDATE subscriber_name = '$subscriber_name'";

    if(!mysql_query($sqlName)){
        die('Error: ' .mysql_error());
    }

Note: Thanks for all the advices about sql injection but the question wasn't about security.

maeq
  • 1,073
  • 1
  • 12
  • 23
  • *"Is this the correct way to update / insert records using PHP?"* - have you tried it? gotten errors? – Funk Forty Niner Aug 19 '15 at 12:34
  • Your question title is rather contradictive to the question. What you should be using are prepared statements and drop the old `mysql_` stuff. – Funk Forty Niner Aug 19 '15 at 12:37
  • [Your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Jay Blanchard Aug 19 '15 at 12:48
  • If you can, you should [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) [statements](http://php.net/manual/en/pdo.prepared-statements.php) instead, and consider using PDO, [it's really not hard](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Aug 19 '15 at 12:48
  • @JayBlanchard and Fred, thanks for the advices about stop using mysql_ functions. Had no idea... I started learning PHP some days ago :) – maeq Aug 19 '15 at 13:29
  • You should submit your code for review! – DLastCodeBender Aug 19 '15 at 14:10

3 Answers3

1

INSERT .. ON DUPLICATE KEY UPDATE may be just what you are looking for. see https://dev.mysql.com/doc/refman/5.0/en/insert-on-duplicate.html

You need to have an unique constraint on subscriber_email though.

cezaminio
  • 26
  • 2
0

NO. It is not SQL-injection-proof. If somwone enters 'a or 1 = 1 ' as subscriber_email, he will update all of your entries. have a look on Wikipedia: SQL-Injection and PHP-Doc to prepared statements

inetphantom
  • 2,498
  • 4
  • 38
  • 61
0

First off, at least use sqli instead of sql. here is a good documentation on sql injection in PHP http://php.net/manual/en/security.database.sql-injection.php

Also, if you're not sure about the inputs that are coming from the user (e.g. email address), then you can use an email parser http://php.net/manual/en/ref.mailparse.php (server side). It's a good practice to validate the value on both client and server side.

Remember, more rules for the form entries means more security and thereby less sql injection vulnerability.

Ray
  • 781
  • 2
  • 17
  • 42