2

hello guys im trying to script a register form for my database and i came with this code >> so can anyone help ?

Private Sub Button2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.Click
    Dim cn As New SqlConnection
    Dim cmd As New SqlCommand
    Dim dr As SqlDataReader

    cn.ConnectionString = "Server=localhost;Database=test;Uid=sa;Pwd=fadyjoseph21"
    cmd.Connection = cn
    cmd.CommandText = "INSERT INTO test2(Username,Password) VALUES('" & TextBox1.Text & "','" & TextBox2.Text & "')"
    cn.Open()
    dr = cmd.ExecuteReader
    If dr.HasRows Then
        MsgBox("You're already registered")
    Else
        MsgBox("Already registered")
    End If
End Sub
Joe
  • 51
  • 1
  • 1
  • 5
  • 1
    You have to set the `CommandText` *before* you try to execute the command, not *after*. It's not even really clear what you're *trying* to do here, since an `INSERT` command isn't going to return rows for a `DataReader`. Also, your code is *wide open* to **SQL injection**. – David Apr 05 '16 at 14:06
  • Can u edit the code for me as i dont get what u said .. @David – Joe Apr 05 '16 at 14:10
  • @Joe I've posted an answer which includes an example of David's comment about SQL Injection. The short answer here is that you're attempting to edit the SqlCommand's SQL query while you're still using the SqlDataReader with it. – Ortund Apr 05 '16 at 14:27

5 Answers5

3

Edit your Code in this way..

cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES('" & TextBox1.Text & "' , '" & TextBox2.Text & "')"
cn.Open()
cmd.ExecuteNonQuery() 
cn.Close()

Insert will not retrieve any records it's a SELECT statement you want to use .I'll suggest you use stored procedures instead to avoid Sql-Injections.

Community
  • 1
  • 1
jamiedanq
  • 967
  • 7
  • 12
  • as @rt2800 has stated a could be any column in your table for example "select * from TblToRead where column=somevalue" – jamiedanq Apr 05 '16 at 14:17
  • There are more columns in the INSERT statement than values specified in the VALUES clause. The number of values in the VALUES clause must match the number of columns specified in the INSERT statement. – Joe Apr 05 '16 at 14:19
  • @Joe do you want to return some data from a table or do you just want to insert into a table? – jamiedanq Apr 05 '16 at 14:21
  • i used select statment and when i try to register with ex da as username and da as password it gives me this error > Invalid column name 'da'. Invalid column name 'da'. @jamiedanq – Joe Apr 05 '16 at 14:24
  • i want to add .. as i need to register not login . – Joe Apr 05 '16 at 14:25
  • @if you want to add then you don't need the select statement, you need an insert like you were using before – jamiedanq Apr 05 '16 at 14:27
  • well i used insert but i got this error >There are more columns in the INSERT statement than values specified in the VALUES clause. The number of values in the VALUES clause must match the number of columns specified in the INSERT statement. @jamiedanq – Joe Apr 05 '16 at 14:30
  • remove the 'and' and put a comma ',' like I've done above. I've changed the code – jamiedanq Apr 05 '16 at 14:32
  • Instead of ('" & TextBox1.Text & "' and '" & TextBox2.Text & "') use ('" & TextBox1.Text & "' , '" & TextBox2.Text & "'). Hope this helps – jamiedanq Apr 05 '16 at 14:36
  • @Joe very well all the best coding, if it helped please mark as right answer – jamiedanq Apr 05 '16 at 14:43
1

ExecuteReader it's for "SELECT" queries, that helps to fill a DataTable. In this case you execute command before cmd.commandText is defined.

You should have define cmd.commandText before and use ExecuteNonQuery after, like this.

Dim cn As New SqlConnection
Dim cmd As New SqlCommand

cn.ConnectionString = "Server=localhost;Database=test;Uid=sa;Pwd=fadyjoseph21"
cmd.Connection = cn
cn.Open()
cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES('" & TextBox1.Text & "','" & TextBox2.Text & "')"
cmd.ExecuteNonQuery()
cn.Close()
aaa3s
  • 11
  • 2
0

cmd.CommandText should be assigned stored proc name or actual raw SQL statement before calling cmd.ExecuteReader

Update:

Change code as follows

.... cmd.Connection = cn cmd.CommandText = "select * from TblToRead where <filter>" ''This is select query statement missing from your code cn.Open() dr = cmd.ExecuteReader ....

where <filter> will be something like username = "' & Request.form("username') & '" '

rt2800
  • 3,045
  • 2
  • 19
  • 26
0

You're attempting to change the CommandText after you're executing your query.

Try this:

Private cn = New SqlConnection("Server=localhost;Database=test;UID=sa;PWD=secret")

Private Sub Button2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.Click
    Dim cmd As New SqlCommand
    cmd.CommandText = "select * from table1" ' your sql query selecting data goes here
    Dim dr As SqlDataReader
    cmd.Connection = cn
    cn.Open()
    dr = cmd.ExecuteReader
    If dr.HasRows = 0 Then
        InsertNewData(TextBox1.Text, TextBox2.Text)
    Else
        MsgBox("Already registered")
    End If
End Sub

Private Sub InsertNewData(ByVal username As String, ByVal password As String)
    Dim sql = "INSERT INTO User_Data(Username,Password) VALUES(@Username, @Password)"

    Dim args As New List(Of SqlParameter)
    args.Add(New SqlParameter("@Username", username))
    args.Add(New SqlParameter("@Password", password))

    Dim cmd As New SqlCommand(sql, cn)
    cmd.Parameters.AddRange(args.ToArray())

    If Not cn.ConnectionState.Open Then
        cn.Open()
    End If

    cmd.ExecuteNonQuery()
    cn.Close()
End Sub

This code refers the INSERT command to another procedure where you can create a new SqlCommand to do it.

I've also updated your SQL query here to use SqlParameters which is much more secure than adding the values into the string directly. See SQL Injection.

The InsertNewData method builds the SQL Command with an array of SQLParameters, ensures that the connection is open and executes the insert command.

Hope this helps!

Ortund
  • 8,095
  • 18
  • 71
  • 139
  • i got many error .. some about args as its protection level .. and one at Of that is at List ( Of Sqlparameter) and one at Private Sub InsertNewData( Dim username As String, Dim password As String) ( Dim is not blue ) – Joe Apr 05 '16 at 14:35
  • my bad, changing from C# to VB doesn't always work well in my head. I've updated the code, please try it again – Ortund Apr 05 '16 at 15:16
  • ExecuteReader: CommandText property has not been initialized got this error when pressing register – Joe Apr 06 '16 at 10:57
  • @Joe I had assumed you already had the command specified before that in your code. I've amended the code sample to include a select command. – Ortund Apr 08 '16 at 10:22
0

The error itself is happening because you're trying to execute a query before you define that query:

dr = cmd.ExecuteReader
'...
cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES('" & TextBox1.Text & "' and '" & TextBox2.Text & "')"

Naturally, that doesn't make sense. You have to tell the computer what code to execute before it can execute that code:

cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES('" & TextBox1.Text & "' and '" & TextBox2.Text & "')"
'...
dr = cmd.ExecuteReader

However, that's not your only issue...

You're also trying to execute a DataReader, but your SQL command doesn't return data. It's an INSERT command, not a SELECT command. So you just need to execute it directly:

cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES('" & TextBox1.Text & "' and '" & TextBox2.Text & "')"
cmd.ExecuteNonQuery

One value you can read from an INSERT command is the number of rows affected. Something like this:

cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES('" & TextBox1.Text & "' and '" & TextBox2.Text & "')"
Dim affectedRows as Int32 = cmd.ExecuteNonQuery

At this point affectedRows will contain the number of rows which the query inserted successfully. So if it's 0 then something went wrong:

If affectedRows < 1 Then
    'No rows were inserted, alert the user maybe?
End If

Additionally, and this is important, your code is wide open to SQL injection. Don't directly execute user input as code in your database. Instead, pass it as a parameter value to a pre-defined query. Basically, treat user input as values instead of as executable code. Something like this:

cmd.CommandText = "INSERT INTO User_Data(Username,Password) VALUES(@Username,@Password)"
cmd.Parameters.Add("@Username", SqlDbType.NVarChar, 50).Value = TextBox1.Text
cmd.Parameters.Add("@Password", SqlDbType.NVarChar, 50).Value = TextBox2.Text

(Note: I guessed on the column types and column sizes. Adjust as necessary for your table definition.)

Also, please don't store user passwords as plain text. That's grossly irresponsible to your users and risks exposing their private data (even private data on other sites you don't control, if they re-use passwords). User passwords should be obscured with a 1-way hash and should never be retrievable, not even by you as the system owner.

David
  • 208,112
  • 36
  • 198
  • 279