1

I have a login system, that will have an option to change a user password if forgotten once they answer their security question correctly.

Everything is working fine, except I'm unsure how to determine if it was successful or not.

If TextBox1.Text = TextBox2.Text Then
            Dim conn As MySqlConnection
            conn = New MySqlConnection("server=localhost; user id=root; password=; database=testing")
            Dim username As Boolean = True
            conn.Open()
            Dim sqlquery As String = "UPDATE user SET password='" & TextBox1.Text & "' WHERE id='" & frmLogin.CurrentUserID & "';"
            Dim data As MySqlDataReader
            Dim adapter As New MySqlDataAdapter
            Dim command As New MySqlCommand
            command.CommandText = sqlquery
            command.Connection = conn
            adapter.SelectCommand = command
            data = command.ExecuteReader
            Dim i As Integer = command.ExecuteNonQuery()
            If (i > 0) Then
                MessageBox.Show("Success!")
            Else
                MessageBox.Show("Failed!")
            End If
            data.Close()
            conn.Close()


        Else
            MsgBox("Passwords must match!")
        End If

It should show a message box saying "Success!" if it worked and "Failed!" if not. I'm getting an error --> 'There is already an open DataReader associated with this Connection which must be closed first.'

Anthony
  • 37
  • 4

1 Answers1

3

That's not the correct way to execute an UPDATE query.
You don't need to call any ExecuteReader and you don't need any MySqlDataAdapter. Of course leaving the reader open triggers your actual error because the connection cannot execute any other command until it is freed from serving the reader. But you have also three big problems in your code not immediately evident.

You should always execute parameterized queries and never create sql commands concatenating strings from the user input. (Search about Sql Injection).

You should always enclose your MySqlConnection in a using statement block to correctly dispose it.

Using conn as MySqlConnection = New MySqlConnection("server=localhost; user id=root; password=; database=testing")
    Dim username As Boolean = True
    conn.Open()
    Dim sqlquery As String = "UPDATE user SET password=@pass WHERE id=@id"
    Dim command As New MySqlCommand
    command.CommandText = sqlquery
    command.Connection = conn
    command.Parameters.Add("@pass", MySqlDbType.VarChar).Value = TextBox1.Text
    command.Parameters.Add("@id", MySqlDbType.Int32).Value = frmLogin.CurrentUserID
    Dim i As Integer = command.ExecuteNonQuery()
    If (i > 0) Then
        MessageBox.Show("Success!")
    Else
        MessageBox.Show("Failed!")
    End If
End Using

There is another problem but this is more complex to solve. Storing passwords in clear text inside a database is considered a very high security risk. You should learn how to Hash and Salt passwords before storing them in the database

Steve
  • 213,761
  • 22
  • 232
  • 286
  • 1
    I do agree about the password storage (salting and hashing), I'm just beginning learning data driven apps, so I'm breaking it up into smaller portions rather than everything at once. I will be implementing that before deploying application. On a side note, I tried to implement your suggestion, however I'm receiving 'Int' is not a member of MySqlDbType. Should I change to Int 16,24, or 32? – Anthony Apr 21 '19 at 19:57
  • Oh, yes, its is Int32 for int values. Sorry, but writing manually here without of any kind of Intellisense to help – Steve Apr 21 '19 at 19:59
  • Thanks so much, I really appreciate your help. I noticed you removed the conn.close() in your example. Was that a mistake or should I still have it right above the End Using? – Anthony Apr 21 '19 at 20:00
  • 1
    No, that's not an error and it is another benefit of the Using Statement. At the exit point the disposable object is disposed and for a IDbConnection this means Close/Dispose – Steve Apr 21 '19 at 20:01