Optimization?

11 views
Skip to first unread message

Michael

unread,
Apr 12, 2003, 9:07:47 PM4/12/03
to
Does anyone know what i can do here to optimize my code?
Or is it as optimized as it can get? It seems kinda
slugish when people login correctly. Also i need this to
carry on through a few pagse on a member login, so would
passing cookies be better or session login?

Thanks for any help!!!!!!!!!! =)

<%
Response.Expires = -1000 'Make sure the browser doesnt
cache this page
Response.Buffer = True 'enables response.redirect to work

If Request.Form("valuepassed") ="true" Then
CheckLoginForm
Else
ShowLoginForm
End If

Sub CheckLoginForm
Dim myconn, blnLoggedIn, myconn2, myconn3, times, exceeded
Dim strfirst, strlast, strpassword, strdate, expired,
myconn4
Set myconn = Server.CreateObject("ADODB.Connection")
myconn.Open = "DSN=xxx;uid=xxx;pwd=xxx"
Set myconn2 = Server.CreateObject("ADODB.Connection")
myconn2.Open = "DSN=xxx;uid=xxx;pwd=xxx"
Set myconn3 = Server.CreateObject("ADODB.Connection")
myconn3.Open = "DSN=xxx;uid=xxx;pwd=xxx"
Set myconn4 = Server.CreateObject("ADODB.Connection")
myconn4.Open = "DSN=xxx;uid=xxx;pwd=xxx"

strfirst = Request.Form("firstn")
strlast = Request.Form("lastn")
strpassword = Request.Form("password")
strdate = DateSerial("1998","12","31")
exceeded = "3"

Dim rsTest
Set rsTest = myconn.execute("SELECT id FROM Upgrade WHERE
fname='" & strfirst & "' AND lname='" & strlast & "' AND
serial='" & strpassword & "';")
If rsTest.EOF Then '''NO RECORDS MATCH. USER DID NOT LOG
IN CORRECTLY
blnLoggedIn = False
Response.Redirect "http://www.errorpage.com"
Else

Set expired = myconn2.execute("SELECT id FROM Upgrade
WHERE fname='" & strfirst & "' AND lname='" & strlast
& "' AND serial='" & strpassword & "' AND date >='" &
strdate & "';")
If expired.EOF Then '''THE TIME LIMIT IS PAST
blnLoggedIn = False
Response.Redirect "http://www.yourexpired.com"
Else

Set times = myconn3.execute("SELECT id FROM Upgrade WHERE
fname='" & strfirst & "' AND lname='" & strlast & "' AND
serial='" & strpassword & "' AND date >='" & strdate & "'
AND downloads <='" & exceeded & "';")
If times.EOF Then '''EXCEEDED DOWNLOAD LIMIT
blnLoggedIn = False
Response.Redirect "http://www.exceededdownload.com"

Else '''EVERYTHING PASSED PROCEEDE WITH DOWNLOAD
blnLoggedIn = True
myconn4.execute("UPDATE Upgrade set downloads =
(downloads + 1) WHERE fname='" & strfirst & "' AND
lname='" & strlast & "' AND serial='" & strpassword & "'
AND date >='" & strdate & "';")
Response.Redirect "http://www.yourgood.com"

rsTest.Close
Set rsTest = Nothing

expired.Close
Set expired = Nothing

times.Close
Set times = Nothing

ShowLoginForm
End If
End If
End If
End Sub
%>

Stuart Palmer

unread,
Apr 13, 2003, 3:39:29 AM4/13/03
to
SELECT id FROM Upgrade WHERE fname='" & strfirst & "' AND lname='" & strlast
& "' AND serial='" & strpassword

Looks to me you could you this single SQL statement and put this into an RS
and compare values in if statements to process the form

Not tested but might like to try something like this:-

Sub CheckLoginForm
Dim myconn, blnLoggedIn, times, exceeded


Dim strfirst, strlast, strpassword, strdate, expired

Set myconn = Server.CreateObject("ADODB.Connection")
myconn.Open = "DSN=xxx;uid=xxx;pwd=xxx"

myconn2.Open = "DSN=xxx;uid=xxx;pwd=xxx"

strfirst = Request.Form("firstn")


strlast = Request.Form("lastn")
strpassword = Request.Form("password")
strdate = DateSerial("1998","12","31")
exceeded = "3"

Set objRS = myconn.execute("SELECT id FROM Upgrade WHERE fname='" &


strfirst & "' AND lname='" & strlast & "' AND serial='" & strpassword &
"';")

If objRS.EOF Then '''NO RECORDS MATCH. USER DID NOT LOG IN CORRECTLY


blnLoggedIn = False
Response.Redirect "http://www.errorpage.com"
Else

If objRS("date") >= strdate Then '''THE TIME LIMIT IS PAST


blnLoggedIn = False
Response.Redirect "http://www.yourexpired.com"
Else

If objRS("downloads") >= exceeded Then


blnLoggedIn = False
Response.Redirect "http://www.exceededdownload.com"
Else '''EVERYTHING PASSED PROCEEDE WITH DOWNLOAD
blnLoggedIn = True

myconn2.execute("UPDATE Upgrade set downloads = (downloads + 1) WHERE


fname='" & strfirst & "' AND lname='" & strlast & "' AND serial='" &
strpassword & "' AND date >='" & strdate & "';")
Response.Redirect "http://www.yourgood.com"

objRS.Close
Set objRS= Nothing

ShowLoginForm
End If
End If
End If
End Sub

hope it makes sense, and it follows what you are trying to do.
Could even get rid of conn2 and use an RS update, like I said, not tested,
but defo opening 4 conns is slooooow.

Stu


Egbert Nierop (MVP for IIS)

unread,
Apr 13, 2003, 4:36:39 AM4/13/03
to
"Michael" <mknut...@hotmail.com> wrote in message
news:023a01c30159$18aa31b0$a101...@phx.gbl...

> Does anyone know what i can do here to optimize my code?
> Or is it as optimized as it can get? It seems kinda
> slugish when people login correctly. Also i need this to
> carry on through a few pagse on a member login, so would
> passing cookies be better or session login?
>
> Thanks for any help!!!!!!!!!! =)

another optimization (by 20% or so)
is to not use DSNs because that's ODBC and giving you only an overhead
Use this
Provider=sqloledb;data source=server;user id=blah;passwords=***;initial
catalog=yourdb

And are you sure you need 4x a dbms connection?

--
compatible web farm Session replacement for Asp and Asp.Net
http://www.nieropwebconsult.nl/asp_session_manager.htm

Bob Barrows

unread,
Apr 13, 2003, 10:43:49 AM4/13/03
to
Michael wrote:
> Sub CheckLoginForm
> Dim myconn, blnLoggedIn, myconn2, myconn3, times, exceeded
> Dim strfirst, strlast, strpassword, strdate, expired,
> myconn4
> Set myconn = Server.CreateObject("ADODB.Connection")
> myconn.Open = "DSN=xxx;uid=xxx;pwd=xxx"

Mistake #1 - don't use ODBC - it's obsolete and does not have the
optimization features built into the latest OLEDB providers. See this page
for connection string examples using OLEDB:
www.able-consulting.com/ado_conn.htm

Mistake #2 - you are opening this connection before you need it. Your goal
should be to open a connection immediately before you need it, and close it
just as soon as you're finished with it. This will allow OLEDB Session
Pooling to optimize connections and resources usage.


> Set myconn2 = Server.CreateObject("ADODB.Connection")
> myconn2.Open = "DSN=xxx;uid=xxx;pwd=xxx"
> Set myconn3 = Server.CreateObject("ADODB.Connection")
> myconn3.Open = "DSN=xxx;uid=xxx;pwd=xxx"
> Set myconn4 = Server.CreateObject("ADODB.Connection")
> myconn4.Open = "DSN=xxx;uid=xxx;pwd=xxx"

Mistakes #3-8. There is never a need to open multiple connections unless
each one points to a different data source. Again - you are opening them
long before they are needed.

>
> strfirst = Request.Form("firstn")
> strlast = Request.Form("lastn")
> strpassword = Request.Form("password")
> strdate = DateSerial("1998","12","31")
> exceeded = "3"
>
> Dim rsTest

Here is where you should open the connection

> Set rsTest = myconn.execute("SELECT id FROM Upgrade WHERE
> fname='" & strfirst & "' AND lname='" & strlast & "' AND
> serial='" & strpassword & "';")
> If rsTest.EOF Then '''NO RECORDS MATCH. USER DID NOT LOG
> IN CORRECTLY
> blnLoggedIn = False

Mistake #9 - Always explicitly close and destroy your ADO objects:

rsTest.Close
Set rsTest=nothing
myconn.close
set myconn=nothing

> Response.Redirect "http://www.errorpage.com"
> Else
>
Mistakes 10 - ?: You don't need to go back to the database. You should have
asked for [date] and downloads in the initial query:
Set rsTest = myconn.execute("SELECT [date],downloads FROM Upgrade ..."

That would enable you to look at their values now instead of going back to
the database:

'first stuff the data into an array so you can kill the recordset:
dim arResults
arResults = rsTest.GetRows
rsTest.close
set rsTest = nothing
'now check the date and download values:
If cdate(arResults(0,0)) <= strdate then
Response.Redirect "http://www.yourexpired.com"
elseif arResults(1,0) >= exceeded then
Response.Redirect "http://www.exceededdownload.com"
else
blnLoggedIn = True
myconn.execute("UPDATE Upgrade set downloads =


(downloads + 1) WHERE fname='" & strfirst & "' AND
lname='" & strlast & "' AND serial='" & strpassword & "'

AND date >='" & strdate)
myconn.close
set myconn=nothing
Response.Redirect "http://www.yourgood.com"
end if

Are you using SQL Server? (it looks like it - you're not using # date
delimiters.) If so, you will see a quantum leap in performance if you do all
the above checks in a single stored procedure. Use QA to run this script in
your database on your server (this is all untested - and I may have made bad
guesses as to your datatypes and lengths):

CREATE PROCEDURE ValidateLogin
(
@fname varchar(50),
@lname varchar(50),
@pwd varchar(50)
) AS
SET NOCOUNT ON
DECLARE @lastlogin datetime, @downloads int

SELECT @logindate = [date], @downloads = downloads
FROM Upgrade WHERE fname = @fname AND
lname = @lname AND serial = @pwd

IF @@ROWCOUNT = 0
BEGIN
RAISERROR
('Invalid login',16,1)
Return 0
END

IF COALESCE(@logindate,'19981231') < '19981231'
BEGIN
RAISERROR
('Time Expired',16,2)
Return 0
END


IF COALESCE(@downloads ,0) > 3
BEGIN
RAISERROR
('Downloads Exceeded',16,3)
Return 0
END

UPDATE Upgrade set downloads = @downloads + 1
WHERE fname = @fname AND
lname = @lname AND serial = @pwd
/*I'm not sure the following is needed: will there be
multiple records for a user?*/
AND [date] >= '19981231'

Return 0
Go

Then, in your asp page:

Sub CheckLoginForm
Dim myconn, sErr,blnLoggedIn
Dim strfirst, strlast, strpassword
'initialize the strfirst,etc variables as before, then:
sErr = ""


Set myconn = Server.CreateObject("ADODB.Connection")

myconn.Open = "provider=sqloledb;data source=servername;" & _
"initial catalog=databasename;user name=xxx;password=xxx"

On Error Resume Next
myconn.ValidateUser strfirst,strlast,strpassword
if myconn.errors.count > 0 then
sErr= myconn.errors(0).description
end if

'immediately close and destroy myconn - we are done with it
myconn.close
set myconn = nothing

if len(sErr) > 0 then
select case sErr
case "Invalid String"
Response.Redirect "http://www.errorpage.com"
case "Time Expired"
Response.Redirect "http://www.yourexpired.com"
case "Downloads exceeded"
Response.Redirect "http://www.exceededdownload.com"
case else
'redirect to an "unexpected error" page, perhaps passing the
'sErr string in the querystring
else
Response.Redirect "http://www.yourgood.com"
end if
end sub

HTH,
Bob Barrows
PS. "date" is a bad column name: first of all, it's a reserved word, so
using it as an object name can lead to hard-to-debug problems, unless you
remember to always surround it with brackets []. Additionally, it's not very
descriptive, is it? What date is stored? The last login date? The date the
user account was created? Will you remember 2 years from now when you have
to come back and maintain this code?


Michael

unread,
Apr 13, 2003, 11:28:46 AM4/13/03
to

ADODB.Recordset error '800a0cc1'

Item cannot be found in the collection corresponding to
the requested name or ordinal.

that is the error i get when i do it that way. Like the
objRS doesn't work past the first line...

>.
>

Egbert Nierop (MVP for IIS)

unread,
Apr 14, 2003, 5:54:24 AM4/14/03
to
Your login is open to SQL Escape Code attacks.
Have each parameter encapsulated by

SELECT id FROM Upgrade WHERE fname='" & replace(strfirst,"'","''") & "' AND
lname='" & Replace(strlast, "'", "''")
& "' AND serial='" & Replace(strPassWord, "'", "''")

And b.t.w. for the real optimization, I'd use a stored proc in SQL server
and call it by using the ADODB.Command object. The recordset is a huge
overhead here (for just one record)...

--
compatible web farm Session replacement for Asp and Asp.Net
http://www.nieropwebconsult.nl/asp_session_manager.htm

"Michael" <mknut...@hotmail.com> wrote in message
news:017001c301d1$602361b0$2f01...@phx.gbl...

Reply all
Reply to author
Forward
0 new messages