#1
  1. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jun 2012
    Posts
    17
    Rep Power
    0

    Help me debug this if statement...


    <%
    saleid = Request.QueryString("saleid")

    set conn = server.CreateObject ("ADODB.Connection")
    conn.Open "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & server.MapPath ("users.mdb")
    set rs = server.CreateObject ("ADODB.Recordset")
    rs.Open "SELECT * FROM sales", conn, 3, 3
    do while not rs.EOF
    if rs.Fields("id") = saleid then
    dim strsql3
    strsql3 = "DELETE * from Sales where id = " & saleid
    Response.Write(strsql3)
    rs.Execute strsql3
    else
    Response.Write(rs.Fields("id"))
    Response.Write(saleid)

    end if
    rs.MoveNext
    loop
    set rs = nothing
    set conn = nothing
    'Response.Redirect("addsales.asp")
    Response.Write(strsql3)
    %>

    The fields ids in the database are 9 and 10. The sale ids I've tried are also 9 and 10.

    When I run this, it prints 99109. Obv those Response.Writes are my failed debugging attempt.

    Gabriel
  2. #2
  3. No Profile Picture
    Stumpier old Moderator
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jun 2003
    Posts
    14,409
    Rep Power
    4538
    First, please use the forum CODE bb tags for readability of posted code. Refer to the forum faq's for more info.

    First thing that jumps out you should move your dim strsql3 outside of your do loop. And I'd build a list of the id's to delete inside the loop, then process that list after the initial loop has completed.
    ======
    Doug G
    ======
    It is a truism of American politics that no man who can win an election deserves to. --Trevanian, from the novel Shibumi
  4. #3
  5. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jun 2012
    Posts
    17
    Rep Power
    0
    Originally Posted by Doug G
    First, please use the forum CODE bb tags for readability of posted code. Refer to the forum faq's for more info.

    First thing that jumps out you should move your dim strsql3 outside of your do loop. And I'd build a list of the id's to delete inside the loop, then process that list after the initial loop has completed.
    It will only be 1 id. Does it make more sense to do it a completely different way then?
  6. #4
  7. No Profile Picture
    Stumpier old Moderator
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jun 2003
    Posts
    14,409
    Rep Power
    4538
    If all you want to do is delete a single id you don't need to loop through all the records, just do the delete statement and be done.

    It's very very dangerous to pass unfiltered user input as part of your sql. Anything that comes from a browser can be hacked by malicious users. You should validate the sale id value before using in your sql.

    Also if your table has many records doing a select * without a limiting where clause in the sql can overload the server asp. Imagine your loop if you had a few hundred or thousand sales records.
    ======
    Doug G
    ======
    It is a truism of American politics that no man who can win an election deserves to. --Trevanian, from the novel Shibumi
  8. #5
  9. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jun 2012
    Posts
    17
    Rep Power
    0
    Originally Posted by Doug G
    If all you want to do is delete a single id you don't need to loop through all the records, just do the delete statement and be done.

    It's very very dangerous to pass unfiltered user input as part of your sql. Anything that comes from a browser can be hacked by malicious users. You should validate the sale id value before using in your sql.

    Also if your table has many records doing a select * without a limiting where clause in the sql can overload the server asp. Imagine your loop if you had a few hundred or thousand sales records.
    Wait. So could I just do Select * FROM table where id = saleid?

    Because that'd be way easier.

    How is this vulnerable to malicious users? All they can do is delete a specific record...I understand that'd be a problem, but I'm not sure how I'd validate the saleid that's coming in.

    Thanks again for your time and patience.

    Gabriel
  10. #6
  11. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jun 2012
    Posts
    17
    Rep Power
    0
    Code:
    <% 
    saleid = Request.QueryString("saleid") 
    set conn = server.CreateObject ("ADODB.Connection") 
    conn.Open "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & server.MapPath ("users.mdb") 
    set rs = server.CreateObject ("ADODB.Recordset") 
    rs.open "SELECT * FROM Sales ", conn, 3,3 
    strsql3 = "DELETE * from Sales where id = " & saleid 
    rs.Execute(strsql3) 
    set rs = nothing 
    set conn = nothing 
    'Response.Redirect("addsales.asp") 
    Response.Write(strsql3) 
    %>
    I am now getting the following error:

    Microsoft VBScript runtime error '800a01b6'

    Object doesn't support this property or method: 'rs.Open'

    /deletesales.asp, line 7

    But when I write the SQL statement, it appears how I want it to.

    Gabriel
  12. #7
  13. No Profile Picture
    Stumpier old Moderator
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jun 2003
    Posts
    14,409
    Rep Power
    4538
    The error message doesn't match you code. Are you missing a newline or some quotes somewhere? "rs.open" shouldn't be showing up as a property or method name.

    Also you don't have to select the record prior to deleting it, although you may want to verify you have the right record to delete prior to the actual deletion.

    From an earlier comment, google for "sql injection" and "cross-site scripting" to find out more about common web attack vectors.
    ======
    Doug G
    ======
    It is a truism of American politics that no man who can win an election deserves to. --Trevanian, from the novel Shibumi
  14. #8
  15. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jun 2012
    Posts
    17
    Rep Power
    0
    Originally Posted by Doug G
    The error message doesn't match you code. Are you missing a newline or some quotes somewhere? "rs.open" shouldn't be showing up as a property or method name.

    Also you don't have to select the record prior to deleting it, although you may want to verify you have the right record to delete prior to the actual deletion.

    From an earlier comment, google for "sql injection" and "cross-site scripting" to find out more about common web attack vectors.
    Sorry, oversight.

    Microsoft VBScript runtime error '800a01b6'

    Object doesn't support this property or method: 'rs.Execute'

    /deletesales.asp, line 9

    And rs.Execute is on line 9, there's a line break in there somewhere.
  16. #9
  17. No Profile Picture
    Stumpier old Moderator
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jun 2003
    Posts
    14,409
    Rep Power
    4538
    I don't see Execute as a adodb recordset method.

    http://msdn.microsoft.com/en-us/libr...=vs.85%29.aspx
    ======
    Doug G
    ======
    It is a truism of American politics that no man who can win an election deserves to. --Trevanian, from the novel Shibumi

IMN logo majestic logo threadwatch logo seochat tools logo