2

Using Dreamweaver CS5 I have added the following Server Behaviour which is working fine.

Question is, do I need to .Close the MM_rsUser1?

The auto-generated code closes the MM_rsUser, but when I tried to close the MM_rsUser1 on the lines before or after where the MM_rsUser is closed, the page fails.

I found this reference for MySql that seems to indicate I may not 'need' to, but as this is my first project, I am trying to learn as many 'good habits' as possible...and since Dreamweaver is generating much of the VB code, I don't want to 'assume' what it does for me is necessarily the best practice today. (The project is adding dynamic data and editing said data to a pre-exising Classic ASP site...my next project will be upgrading it to MVC/C#)

<%
' *** Validate request to log in to this site.
MM_LoginAction = Request.ServerVariables("URL")
If Request.QueryString <> "" Then MM_LoginAction = MM_LoginAction + "?" + Server.HTMLEncode(Request.QueryString)
MM_valUsername = CStr(Request.Form("userid"))
If MM_valUsername <> "" Then
  Dim MM_fldUserAuthorization
  Dim MM_redirectLoginSuccess
  Dim MM_redirectLoginFailed
  Dim MM_loginSQL
  Dim MM_rsUser
  Dim MM_rsUser_cmd
  Dim MM_loginUpdate ' used to execute timestamp to log last successful login for user
  Dim MM_rsUser1 '     also used to execute timestamp as above

  MM_fldUserAuthorization = "accessLevel"
  MM_redirectLoginSuccess = "/sql.asp"
  MM_redirectLoginFailed = "/login.asp"

  MM_loginSQL = "SELECT email, password"
  If MM_fldUserAuthorization <> "" Then MM_loginSQL = MM_loginSQL & "," & MM_fldUserAuthorization
  MM_loginSQL = MM_loginSQL & " FROM table WHERE userid = ? AND pword = ?"
  Set MM_rsUser_cmd = Server.CreateObject ("ADODB.Command")
  MM_rsUser_cmd.ActiveConnection = MM_SQL_STRING
  MM_rsUser_cmd.CommandText = MM_loginSQL
  MM_rsUser_cmd.Parameters.Append MM_rsUser_cmd.CreateParameter("param1", 202, 1, 50, MM_valUsername) ' adVarWChar
  MM_rsUser_cmd.Parameters.Append MM_rsUser_cmd.CreateParameter("param2", 202, 1, 50, Request.Form("password")) ' adVarWChar
  MM_rsUser_cmd.Prepared = true
  Set MM_rsUser = MM_rsUser_cmd.Execute

  If Not MM_rsUser.EOF Or Not MM_rsUser.BOF Then 
    ' username and password match - this is a valid user
    Session("MM_Username") = MM_valUsername
    MM_loginUpdate = "UPDATE table SET lastLoggedIn = { fn NOW() } WHERE userid = '" & MM_valUsername & "'"
    MM_rsUser_cmd.CommandText = MM_loginUpdate
    Set MM_rsUser1 = MM_rsUser_cmd.Execute ' unsure if I have to write an MM_rsUser1.Close somewhere or not, but page fails where I've tried
    If (MM_fldUserAuthorization <> "") Then
      Session("MM_UserAuthorization") = CStr(MM_rsUser.Fields.Item(MM_fldUserAuthorization).Value)
    Else
      Session("MM_UserAuthorization") = ""
    End If
    if CStr(Request.QueryString("accessdenied")) <> "" And true Then
      MM_redirectLoginSuccess = Request.QueryString("accessdenied")
    End If
    MM_rsUser.Close
    Response.Redirect(MM_redirectLoginSuccess)
  End If
  MM_rsUser.Close
  Response.Redirect(MM_redirectLoginFailed)
End If
%>
Community
  • 1
  • 1
Mike A
  • 244
  • 1
  • 10

1 Answers1

0

The code is a little tricky, at first look. I see 2 statements that close MM_rsUser. The Response.Redirect() acts like a return statement, so only one or the other will be executed, even though the IF block tends to make it look otherwise. I think the key for you is that you can't close MM_rsUser1 if you don't enter the IF block, because it never was opened. So I would suggest this:

    If Not MM_rsUser.EOF Or Not MM_rsUser.BOF Then 
        ' username and password match - this is a valid user
        Session("MM_Username") = MM_valUsername
        MM_loginUpdate = "UPDATE table SET lastLoggedIn = '" & NOW() & "' WHERE userid = '" & MM_valUsername & "'"
        MM_rsUser_cmd.CommandText = MM_loginUpdate
        Set MM_rsUser1 = MM_rsUser_cmd.Execute ' unsure if I have to write an MM_rsUser1.Close somewhere or not, but page fails where I've tried
        If (MM_fldUserAuthorization <> "") Then
            Session("MM_UserAuthorization") = CStr(MM_rsUser.Fields.Item(MM_fldUserAuthorization).Value)
        Else
            Session("MM_UserAuthorization") = ""
        End If
        if CStr(Request.QueryString("accessdenied")) <> "" And true Then
            MM_redirectLoginSuccess = Request.QueryString("accessdenied")
        End If
        MM_rsUser1.Close 'close it here
        MM_rsUser.Close
        Response.Redirect(MM_redirectLoginSuccess)
    End If 'Not MM_rsUser.EOF Or Not MM_rsUser.BOF 
    'not open, so don't close it
    MM_rsUser.Close
    Response.Redirect(MM_redirectLoginFailed)
End If 'MM_valUsername <> "" 

Update

Reusing MM_rsUser_cmd is confusing at best, and could be a cause of some of the errors. Change

MM_rsUser_cmd.CommandText = MM_loginUpdate
Set MM_rsUser1 = MM_rsUser_cmd.Execute

to

Dim MM_rsUser_cmd1 
Set MM_rsUser_cmd1 = Server.CreateObject ("ADODB.Command")
MM_rsUser_cmd1.ActiveConnection = MM_SQL_STRING
MM_rsUser_cmd1.CommandText = MM_loginUpdate
MM_rsUser_cmd1.Parameters.Append MM_rsUser_cmd1.CreateParameter("param1", 135, 1, -1, NOW()) ' adDBTimeStamp 
MM_rsUser_cmd1.Parameters.Append MM_rsUser_cmd1.CreateParameter("param2", 202, 1, 50, MM_valUsername) ' adVarWChar
MM_rsUser_cmd1.Prepared = true
Set MM_rsUser1 = MM_rsUser_cmd1.Execute
Mike A
  • 244
  • 1
  • 10
David Gorsline
  • 4,933
  • 12
  • 31
  • 36
  • 1
    Thanks for the reply David. Unfortunately, I tried closing it in the place you suggested (sorry for not being clear in my initial question...forgot MM_rsUser also got closed further down also). Any other thoughts? – Mike A May 10 '12 at 19:21
  • 1
    +1 for trying to learn good habits, and for the planned move to MVC/C#. There's not much more I can suggest without knowing the details of "the page fails." Make sure you test all 3 cases: (1) no user ID entered; (2) user ID (MM_valUsername) and password entered, but not found in the lookup; (3) good user ID and password. And I see one more thing that smells funny; I will update my answer. – David Gorsline May 10 '12 at 19:35
  • I tried to do something similar to what you suggest, basically by copy/pasting the first batch of code for MM_rsUser, but the page wouldn't load at all then. My thought process on 'reusing' the MM_reUser_cmd.Execute was that at the point I placed the two lines you noted above in your Update, I 'thought' that since the connection was already open and had already processed the first SQL statement, that I could reuse it by changing the statement from MM_LoginSQL to MM_loginUpdate. The page only fails after a correct user/password when placed before If (MM_fldUserAuthorization <> "") – Mike A May 10 '12 at 21:06
  • In any event, you have to set the parameters for last-login date and user for MM_loginUpdate. – David Gorsline May 10 '12 at 22:10
  • Will do, thanks! Wasn't aware I had to set parameters for the lastUpdate...I did miss one for the user. Guess I figured since I wasn't putting the "?" in the statement I didn't need to. Thanks! – Mike A May 11 '12 at 13:43