ADO/SQL Inserting into a database issues

Question

simple question but this is causing me alot of hassle.
Im using asp to insert data into a database and the code looks right to me and my peers however it is not working! Here is my asp and my html form:

asp:

if request.form("submitbutton") <> "" then
    set conn=Server.CreateObject("ADODB.Connection") 
    conn.Open ="Driver={SQL Server}; Server=//private; Database=QuizDynamics;   Uid=QuizDynamics; Pwd=//private
    set rs=Server.CreateObject("ADODB.recordset")
    rs.Open "Select * from Teachers", conn


    sql="INSERT INTO Teachers (firstname, password, lastname)"
    sql=sql & " VALUES "
    sql=sql & "('" & Request.Form("firstname") & "',"
    sql=sql & "'" & Request.Form("password") & "',"
    sql=sql & "'" & Request.Form("lastname") & "')"

    on error resume next
    conn.Execute sql,recaffected
    if err<>0 then
        Response.Write("No update permissions!")
    else
        Response.Write("<h3>" & recaffected & " record added</h3>")
    end if
    conn.close
end if

%>

html form:

<form name="teacherReg" action="Registration.asp" method="POST">
    1. First name:<br/><input type="text" name="firstname"><br/><br/>
    2. Last name:<br/><input type="text" name="lastname"><br/><br/>
    3. Desired Username :<br/><input type="text" name="username"><br/><br/>
    4. Desired Password :<br/><input type="password" name="password"><br/>
    5. Confirm Password :<br/><input type="password" name="confirmpassword"><br/>
    <input type="submit" value="submit">
</form>

(Im only inserting firstname, password and lastname at the moment for testing purposes)

Answer 1

There's an idiom for this in Hungarian: choosing between two chairs, you've ended up on the floor. :)

You've got code that's loading an entire table (all umpteen rows and however many columns - we're talking about possibly an enormous amount of data) into a recordset, but then you never do anything with it. Then you've got an Execute statement, to which you're appending a "recaffected" variable which, as far as I can tell, does not have a value. Thus, you're telling the Execute statement to apply the changes to zero records. Then, you close the connection, but you never close the recordset.

If you want to do this insert via opening a recordset (not the recommended method, but sometimes it's easier, especially if you're inserting data into dozens of columns), you'd do something like:

If Request.Form("submitbutton") <> "" Then
    firstname = Request.Form("firstname")
    lastname = Request.Form("lastname")
    password = Request.Form("password")
    '- add code here to validate firstname, lastname, password
    Set conn = Server.Createobject("ADODB.Connection")
    conn.Open "Driver={SQL Server};Server=//private;Database=QuizDynamics;Uid=QuizDynamics;Pwd=//private"
    Set rs = Server.Createobject("ADODB.Recordset")
    sql = "SELECT TOP 0 * FROM Teachers"
    rs.Open SQL,Conn
    rs.AddNew
        rs("firstname") = firstname
        rs("lastname")  = lastname
        rs("password")  = password
    rs.Update
    rs.Close
    Set rs = Nothing
    conn.close
    Set conn = Nothing
End If

(Notice that there's a TOP 0 on the recordset - you don't need to load any rows if all you're doing is adding a new row. You could also do something like "SELECT * FROM Teachers WHERE 1 = 2".)

If you want to do this via an Execute statement, you don't need a recordset at all. Also, if you're gonna tell the Execute how many rows to apply the changes to, make sure you tell it a correct number, but it's better to simply not tell it anything.

If Request.Form("submitbutton") <> "" Then
    firstname = Request.Form("firstname")
    lastname = Request.Form("lastname")
    password = Request.Form("password")
    '- add code here to validate firstname, lastname, password
    Set conn = Server.Createobject("ADODB.Connection")
    conn.Open "Driver={SQL Server};Server=//private;Database=QuizDynamics;Uid=QuizDynamics;Pwd=//private"
    sql = "INSERT INTO Teachers (firstname, password, lastname) VALUES "
    sql = sql & "('" & firstname & "','" & password & "','" & lastname & "')"
    conn.Execute sql
    conn.close
    Set conn = Nothing
End If

Answer 2

Well I think you and your peers need to re-evaluate the fundamentals of programming. This code is possibly the most confusing thing I have ever seen.

It is calling this:

  rs.Open "Select * from Teachers", conn

And I have no idea why. Is it trying to...

1. Verify if someone has valid DB Connection credentials before allowing them to write to the DB?
2. Loop through it and insert the values submitted in your form for ALL teachers in your loop?

The logic here is not clear at all, and either way, is missing some things.

---

Additionally, there are Several issues with the code, if not all of it:

1. Use Request.ServerVariables("REQUEST_METHOD") to check if a form is posted, like Zam suggests. Everything has its use, so it should be used when it can.

2. Always do server side validation on your forms (especially for password/confirm password forms!). Javascript can be disabled on the browser if you are doing solely client-side validation, which means your form will have no validation if submitted without javascript.

3. Again, it's hard to see what your trying to do with that recordset statement, but never loop through a recordset to do an insert in asp unless you really have to. Instead do one SQL statement for this. It will be much faster. You can see an example here at W3Schools

4. If you are trying to authenticate the DB connection string/password, then you should first do that part, then move into the part where you write to your DB. Using ON ERROR RESUME NEXT should be used sparingly, you never know when there should be an error that you should be seeing.

5. You have no validation against SQL Injection. This vulnerability could compromise your entire database, very easily. You can find more information why and how to prevent it at this link on Microsoft.com , but in most cases for SQL server, you want to make sure all ' (single quotes) are escaped, by doubling them '', validate all numbers (id's) with IsNumeric(), etc. before you send them to SQL server. In other words, use Replace(sData,"'","''") on the values being submitted in your sql statement's. Never inject table names, etc, only values into your SQL statement.

6. Never use Adodb.Recordset to execute an Insert/Delete/Update statement. Use Adodb.Command. Again, that's what it's for.

---

That practically sums it up for me, maybe I missed something. But there can be many reasons why your code isn't working, it would just help if you could make yourself a bit clearer why you are calling that Recordset object up top, and I can provide more assistance.

Answer 3

Issue #1.

You don't have control "submitbutton". Change from

<input type="submit" value="submit">

to

<input type="submit" value="submit" name="submitbutton">

I also suggest you to check request method:

If (Request.ServerVariables ("REQUEST_METHOD") = "POST") Then 
         ..... bla bla bla .....