-1

When I click the button, it doesn't update the data in the database. I have 3 textboxes one is for the Enter Username, second is for enter old password and third is enter new password

  Dim con As New MySqlConnection("host=localhost; username=root; password=; database=pawnshop")
    Dim cmd As New MySqlCommand
    Dim dr As MySqlDataReader

    con.Open()
    cmd.Connection = con
    cmd.CommandText = " select password from login where password ='" & txtChange.Text & "'"

    dr = cmd.ExecuteReader
    If dr.HasRows Then

        cmd.Connection = con
        cmd.CommandText = " UPDATE login SET password ='" & txtConfirm.Text & "' where userid = '" & txtUser.Text & "'"

    Else
        MsgBox("Password is not correct")

    End If

2 Answers2

1

PASSWORD is a reserved word for MySql. If you have fields with this name you should remember to always put backticks around that word.

Said that your code needs a rewrite to avoid Sql Injection, parsing problems, resources leaking and also a bit improvement in performances.

Using con = New MySqlConnection(......)
Using cmd = New MySqlCommand()
    con.Open()
    cmd.Connection = con
    cmd.CommandText = "select `password` from login where `password` =@oldp"
    cmd.Parameters.Add("@oldp", MySqlDbType.String).Value = txtChange.Text 
    Dim result = cmd.ExecuteScalar()
    if result IsNot Nothing then
       cmd.Parameters.Clear()
       cmd.CommandText = "UPDATE login SET `password` = @newp where userid = @uid"
       cmd.Parameters.Add("@newp", MySqlDbType.String).Value = txtConfirm.Text 
       cmd.Parameters.Add("@uid", MySqlDbType.Int32).Value = txtUser.Text 
       cmd.ExecuteNonQuery()
    Else
       MsgBox("Password is not correct")

    End If
End Using
End Using
  1. Resource leaks - The Using statement ensures that every disposable object is correctly disposed also in case of exceptions
  2. Sql Injection - Using string concatenation makes easy for an attacker to wreak havoc with your database (Here the example)
  3. Parsing problems. - Again, string concatenation fails if one of your string values contains a single quote.
  4. Performance - Creating a DataReader is troublesome for many reason. If you need to read just one column from one row then ExecuteScalar is faster and doesn't block the connection until you close the reader. (Your code cannot work because the reader keeps the connection busy and you cannot use the connection with the command until you close the reader

Another point to keep present is the clear text password problem. If you store the password of your users in clear text in the database, everyone that has access to the database with any kind of administrative tool (MySqlWorkbench, phpMyAdmin....) could easily learn the passwords of all your users. This is more complex matter and if you search this site you could find a lot of questions/answers about that

Community
  • 1
  • 1
Steve
  • 213,761
  • 22
  • 232
  • 286
0

I don't see any cmd.ExecuteReader for your UPDATE command. Did you missed that or it's just missing from the post. You should execute the query saying

    If dr.HasRows Then

        cmd.Connection = con
        cmd.CommandText = " UPDATE login SET password ='" & txtConfirm.Text & "' where userid = '" & txtUser.Text & "'"
       dr = cmd.ExecuteReader // missing statement
    Else
        MsgBox("Password is not correct")

    End If
Rahul
  • 76,197
  • 13
  • 71
  • 125