Andrew
Andrew

Reputation: 7798

Proper way to perform the INSERT statement from C# to MySQL

I created this function that inserts new records -- I submit query directly to it. My question- is it optimal? It is it fool proof and guaranteed to function normally? If not; please advise.

static String Server = "";
static String Username = "";
static String Name = "";
static String password = "";

static String conString = "SERVER=" + Server + ";DATABASE=" + Name + ";UID=" + Username + ";PASSWORD=" + password + ";connect timeout=500000;Compress=true;";

public bool InsertSQL(String Query)
{
    int tmp = 0;
    try
    {
        using (MySqlConnection mycon = new MySqlConnection(conString))
        {
            using (MySqlCommand cmd = new MySqlCommand(Query, mycon))
            {
                mycon.Open();
                try
                {
                    tmp = cmd.ExecuteNonQuery();
                }
                catch
                {
                    if (mycon.State == ConnectionState.Open)
                    {
                        mycon.Close();
                    }
                }
                mycon.Close();
            }
        }
    }
    catch { return tmp > 0 == true ? true : false; }
    return tmp > 0 == true ? true : false;
}

This is my SQL insert that I create in other function and pass as text to insert function. I am open to all suggestions!

String insertSql = @"INSERT INTO `gps_unit_location`
            (`idgps_unit`,`lat`,`long`,`ip`,`unique_id`,
            `loc_age`,`reason_code`,`speed_kmh`,
            `VehHdg`,`Odometer`,`event_time_gmt_unix`,`switches`, `engine_on_off`, `dt`)
                VALUES
            (
            (Select idgps_unit from gps_unit where serial=" + serial + "),'" + lat + "','" + lon + "','" + IP + "','" + unique_id + @"',
            '" + LocAge_mins + "','" + ReasonCode + "','" + Speed + @"',
            '" + VehHdg + "','" + Odometer + "','" + EventTime_GMTUnix + "','" + Switches + "', '" + engine_on_off + @"', DATE_ADD(NOW(), INTERVAL 1 HOUR))
            ";

Upvotes: 0

Views: 1168

Answers (2)

Brian
Brian

Reputation: 5119

I built this answer using your code as the example. Take note of the following line:

cmd.Parameters.AddWithValue("@queryParam", Query);

It is always a best-practice to code for potential SQL Injection attacks even if they are unlikely to happen.

static String Server = "";
static String Username = "";
static String Name = "";
static String password = "";

static String conString = "SERVER=" + Server + ";DATABASE=" + Name + ";UID=" + Username  + ";PASSWORD=" + password + ";connect timeout=500000;Compress=true;";

public bool InsertSQL(String Query)
{
   int tmp = 0;
   try
   {
      using (MySqlConnection mycon = new MySqlConnection(conString))
      {
         using (MySqlCommand cmd = new MySqlCommand(Query, mycon))
         {
            mycon.Open();
            try
            {
                cmd.Parameters.AddWithValue("@queryParam", Query);
                tmp = cmd.ExecuteNonQuery();
            }

            catch
            {
                if (mycon.State == ConnectionState.Open)
                {
                    mycon.Close();
                }
            }
            mycon.Close();
         }
     }
 }
 catch { return tmp > 0 == true ? true : false; }
 return tmp > 0 == true ? true : false;
}

Upvotes: 3

Justin Pihony
Justin Pihony

Reputation: 67135

By making this so generic, you are leaving yourself open to SQL injection. I am guessing you have to build the query and insert values directly. SQL parameters would be better here, you could potentially pass in a params of SqlParameters, however that would still rely on generic text being sent and still leaves you open to an injection.

Here is a SQL Parameter example

Upvotes: 2

Related Questions