0

Follow up question

Is the way I used the using statement in the following code correct?

else if(updated_password == confirm_password)
{
   using (old_connection = new MySqlConnection("server=localhost;user id=" + old_user + ";database=DB;password=" + old_pass))
   {
       MySqlCommand old_cmd = new MySqlCommand("ALTER USER'" + old_user + "'@'localhost' IDENTIFIED BY'" + confirm_password + "'", old_connection);

       old_cmd.ExecuteNonQuery();
       //old_connection.Close();
       MessageBox.Show("Password changed successfully.", "Info", MessageBoxButtons.OK, MessageBoxIcon.Information);
       //old_connection.Open();
    }
}
pinkfloydx33
  • 11,863
  • 3
  • 46
  • 63
JoeyB
  • 123
  • 6

2 Answers2

1
using(var old_connection = ...)

Would be better. There is no need to save the connection outside of the scope of the using statement

Ori Cohen
  • 185
  • 9
  • I declared the MySqlConnection old_connection; at the beginning of the code. So you say I should remove the declaration and just use var old_connection every time I want to connect to the database? and by using the using() statment is there ever a need to say old_connection.Open(), the part that is commented out? – JoeyB May 17 '20 at 14:20
1

You pretty much are using 'using' correctly however a few small issues with the code.

Ideally you can create a new connection each time instead of using the same variable every time. This will help with scoping issues and less likely to make mistakes such as using an old instance of it. Like:

 using (var connection = new MySqlConnection("server=localhost;user id=" + old_user + ";database=DB;password=" + old_pass))

The connection still needs to be opened in the 'using' statement. 'using' only handles is disposal of the object. So first line inside the 'using' should be:

connection.Open();

Parameters should also be used when creating the 'MySqlCommand' to protect against SQL injection attacks.

Tom Dee
  • 2,516
  • 4
  • 17
  • 25
  • "Parameters should also be used when creating the 'MySqlCommand' to protect against SQL injection attacks." So basically MySqlCommand(query,old_connection) where query is ""ALTER USER'" + old_user + "'@'localhost' IDENTIFIED BY'" + confirm_password + "'"" – JoeyB May 17 '20 at 14:24
  • I declared the MySqlConnection old_connection; at the beginning of the code. So you say I should remove the declaration and just use var old_connection every time I want to connect to the database? and by using the using() statment is there ever a need to say old_connection.Open(), the part that is commented out? – JoeyB May 17 '20 at 14:26
  • Yes you should be creating a new connection object each time and call connection.Open(). Using will close the connection so if there are connections then you'll never open it back up again. Parameters wise the top answer here explains it pretty well: https://stackoverflow.com/questions/13580993/mysqlcommand-command-parameters-add-is-obsolete# – Tom Dee May 17 '20 at 14:28
  • I am still a bit confused. So you say if I want to make another connection I should NOT use var connection (or var old_connection) but rather use another one like var conn(...) and immediately use conn.Open()? – JoeyB May 17 '20 at 14:34
  • For each connection you make it should be: 'using(var connection = new MySqlConnection(...))' so making a new one each time. You shouldn't be making a new MySqlConnection outside a 'using' statement – Tom Dee May 17 '20 at 14:37
  • Understand! I have read abit about SQL injection. My database is installed on my PC ONLY and I am only accessing when I am on my PC using the application, so basically I am accessing the DB offline. DO I still need to implement the SQL injection parameter codes? And if so can you put it in your answer so I can see how to do it correctly, if you have time and are willing? – JoeyB May 17 '20 at 14:52
  • 1
    It isn't necessary as the application will still work but could be a security flaw. I'd say put it in another question as it's not really relevant to "using" statements – Tom Dee May 17 '20 at 15:13
  • But if I implement the code in the MySql connection to prevent the injection then I don't need to implement it everywhere else because before in the event I want to execute any mysql commands I will first have to using the mysql connection (using var connection) which is already secured. So either way they can't inject anything because the login is secured. Is this correct? – JoeyB May 17 '20 at 15:29
  • Nope you'll have to defined and use parameters on the MySqlCommand separately not on the connection – Tom Dee May 17 '20 at 15:35
  • I see so I only apply it when I am using the MySQLCommand and NOT the MySQLConnection? I have tried to use the common SQL injection techniques like x' OR 'x'='x in the username and password but the programs immediately return an error. – JoeyB May 17 '20 at 17:24