...

View Full Version : Request.QueryString in a SELECT statement?



Speedy
11-08-2003, 03:10 PM
Is it possible to use a Request.QueryString in an
SELECT statement in the SQL?

I tried like this:



rs.open "select * from [Request.QueryString("competition")] order by [POINTS] desc, [POS] asc", conn


I searched for something like this on the forum and on google
but couldn't find anything about it, probably there is a much better way than the one I'm trying but hey I'm only trying to learn ;)

I have a link that I want to open a competition


<a href="show.asp?competition=comp1">Competition 1</a><br>
<a href="show.asp?competition=comp2">Competition 2</a>


the links are on a page named comp.asp and
the SQL is on a page named show.asp

/Speedy

A1ien51
11-08-2003, 04:11 PM
you almost had it



rs.open "select * from [" & Request.QueryString("competition") & "] order by [POINTS] desc, [POS] asc", conn


Eric

M@rco
11-09-2003, 09:33 PM
Request.QueryString("FieldName") simply returns a string, and can therefore be appended to any other string in the usual way.

However, you really SHOULD NOT be using querystring values like this without performing some checks first - there are SERIOUS security implications....

http://www.sitepointforums.com/showthread.php?t=60643

Speedy
11-09-2003, 10:15 PM
Thank you guys,

M@rco thanks for introducing me to the SQL security issues,
I'll read them and try to improve my coding skills to be more secure :)

/Speedy

oracleguy
11-10-2003, 06:41 AM
Yes, the risk is quite large, that peticular flaw is called an SQL Injection Attack because someone could alter your query to do something really bad.

jeskel
11-10-2003, 10:58 AM
check your querystring before using the val in the sql statement... Avoid any char that could be interpreted as being then part of your sql statement.
If you expect numeric, then do something like:


if Not IsNumeric(var) then
'do whatever you want (like redirecting your user)
end if

My example just gives you the main idea... There are actually several more and better checks to do to avoid such problems... But since it's not the topic of this thread... ;)

Caffeine
11-12-2003, 11:50 AM
I agree with what's already been said :)

What has not been told is that if you are going to access a request.querystring value more than once, you should put it into a variable. This will make the code more efficient [although you probably will not notice any differance]

Dim qsName
qsName = Request.QueryString("name")
'-- then do some validating

M@rco
11-12-2003, 12:11 PM
Perfectly true, but since that's one of many *basic* optimization techniques which everyone should know by now (and let's not fill this thread with those, there are are plenty of other threads on that topic already) I was focusing on the serious security ramifications of Speedy/A1ien51's code...

;)

Caffeine
11-12-2003, 02:37 PM
Just because everyone *should* know of them doesn't mean they do, and when I saw this snippet it became obvious to me that the author did not know. :)

But yeah, I could have ignored to reply, the difference is real small and these 'mistakes' are made all the time!

Oh well :D

jeskel
11-12-2003, 02:49 PM
As the ancient japanese sword masters used to say: "perfection can only be reached through repetition". I guess we could adapt this quote to the present discussion. I think it's always good to remember people that kind of details, or just give them a link where those basic things have been discussed. People are not supposed to make those mistakes like they are supposed to have read all the threads related to their question before asking it... huh... realistic? Well... my point is just that it doesn't cost much and that it can be helpfull :)

P.S: if it comes to something discussed in a sticky, my point of view is obviously not the same.....;)

<edit>
welll I don't wanna be misunderstood... A perfect forum would obvisouly a forum where all those details would be known. But it will never be the case. My ASP level is quite low I guess, so I'm happy to help with those details :) ...which doesn't mean that some higher powerd ASP masters have to do it ;)
</edit>

M@rco
11-13-2003, 12:58 AM
Caffeine & bouchel - I don't want to be misunderstood either!! I wasn't dissing anyone's contribution to the thread, merely commenting that Speedy (and A1ien51) would IMHO benefit most from being aware of the *serious* problems with their approach, and that basic optimization techniques - important as they are - would be best left for another thread (of which there are doubtless many already).

However, that's just my opinion. ;)

whammy
11-13-2003, 07:04 AM
I have functions designed specifically to clean up data. Most of you have seen the old versions of my validation/formatting/etc. functions, however I'm giving them an overhaul (to get rid of unnecessary code, make it more understandable, etc.).

For instance:


Function ExtractAlphaNumeric(ByVal str)
If IsNull(str) Then Exit Function
Dim objRegEx
Set objRegEx = New RegExp
objRegEx.Pattern = "[^a-zA-Z0-9]"
ExtractAlphaNumeric = objRegEx.Replace(str,"")
End Function

Function ExtractNumbers(ByVal str)
If IsNull(str) Then Exit Function
Dim objRegEx
Set objRegEx = New RegExp
objRegEx.Pattern = "\D"
ExtractNumbers = objRegEx.Replace(str,"")
End Function

Function ExtractWordChars(ByVal str)
If IsNull(str) Then Exit Function
Dim objRegEx
Set objRegEx = New RegExp
objRegEx.Pattern = "\W"
ExtractWordChars = objRegEx.Replace(str,"")
End Function

Function ExtractWordCharsAndSpaces(ByVal str)
If IsNull(str) Then Exit Function
Dim objRegEx
Set objRegEx = New RegExp
objRegEx.Pattern = "[^A-Za-z0-9_ ]"
ExtractWordCharsAndSpaces = objRegEx.Replace(str,"")
End Function


In my experience, these functions (and related ones) are important regarding SQL injection attacks (not to mention providing clean data to your client easily!).

There are also ways you can help avoid these problems using Stored Procedures with parameters and error checking in SQL, and I could go on and on about this, so I'll stop after the next paragraph.

Really it's up to you to decide what characters must be allowed in your data, while keeping in mind ways that malicious users could compromise not only your data but your application.

EVERY field that you request could potentially be a security risk, regardless of the request method (GET or POST), since the client can modify them using JavaScript or VBScript.

For example, don't use sequential numbers (i.e. a primary key in a database) to update records using a hidden field, I see that error made often.

A nine digit number has 1 billion combinations.

A nine digit string (including alphabetical characters and numbers) has over 3.6 quadrillion combinations.

If you are able to use salt (i.e. another variable that is hard to guess, like an email address or last name), then the odds turn astronomically in your favor (beyond my mathematical understanding, anyway), and against the would-be malicious user.

whammy
11-13-2003, 07:28 AM
P.S. These same functions are much prettier in C# and JavaScript... :eek:

M@rco
11-13-2003, 10:39 AM
Originally posted by whammy
P.S. These same functions are much prettier in C# and JavaScript... :eek: I'm sure they are, but there's plenty of room for improvement here... you can make the above functions into one-liners by modularising your code just a little:
Function RegExReplace(InputString, MatchPattern, ReplacePattern)
Dim objRegEx
Set objRegEx = New RegExp
objRegEx.Pattern = MatchPattern
RegExReplace= objRegEx.Replace(InputString, ReplacePattern)
Set objRegEx = Nothing
End Function

Function ExtractWordCharsAndSpaces(ByVal str)
If IsNull(str) Then ExtractWordCharsAndSpaces = RegExReplace(str, "[^A-Za-z0-9_ ]", "")
End Function
;)

jeskel
11-13-2003, 12:11 PM
Originally posted by M@rco
Caffeine & bouchel - I don't want to be misunderstood either!! I wasn't dissing anyone's contribution to the thread, merely commenting that Speedy (and A1ien51) would IMHO benefit most from being aware of the *serious* problems with their approach, and that basic optimization techniques - important as they are - would be best left for another thread (of which there are doubtless many already).

you're totally right actually... I didn't see it that way. You're even more right that speedy doesn't seem to care a lot about it since he doesn't post anything about it ;) btw I know you weren't dissing, I've never thought that (not your style)

Originally posted by whammy

For example, don't use sequential numbers (i.e. a primary key in a database) to update records using a hidden field, I see that error made often.

hum...... seems like one of those common mistakes that I'm still making :o That will make a thread of its own pretty soon.

Caffeine
11-13-2003, 01:52 PM
Hey,
first of, maybe I am an optimize-freak and want everyone else to care about that area too :) I'm trying to hold it back when I see these things, but sometimes I fail [obviously] :o :p


whammy, does one not have to clean up the objects ?
In your code I see no object-cleanups

ie
Set objRegEx = New RegExp
...
Set objRegEx = Nothing

or does that only apply to 3rd party tools, and not to the built in objects of VBScript ?
:confused:

whammy
11-13-2003, 03:11 PM
From http://www.w3schools.com:



Lifetime of Variables

How long a variable exists is its lifetime.

When you declare a variable within a procedure, the variable can only be accessed within that procedure. When the procedure exits, the variable is destroyed. These variables are called local variables. You can have local variables with the same name in different procedures, because each is recognized only by the procedure in which it is declared.

If you declare a variable outside a procedure, all the procedures on your page can access it. The lifetime of these variables starts when they are declared, and ends when the page is closed.


I've always assumed this to be true with local objects, but you may be right. I'm going to test a bit and find out. :)

A1ien51
11-13-2003, 03:27 PM
When working with ASP....destory your objects! It is easier on the server! The other programmers here always yell at me for not closing my objects ASAP.

When working with .NET, objects get destoryed after making DataSets on their own if I remember correctly, so only thing you need to destroy is the connection.....

M@rco
11-13-2003, 04:55 PM
Yup, you're all quite right - you should ALWAYS destroy objects explicitly, which includes calling any cleanup methods/subs that the object exposes (e.g. .Close for ADO Connections / Recordsets) . DON'T assume that ASP/VBScript will do it for you - there are plenty of flaws in the way that the grabage.

Here's something that I wrote and use in every script to kill my objects explicitly:
Function Kill(byref Obj)
Select Case True
Case IsObject(Obj)
Select Case LCase(TypeName(Obj))
Case "recordset", "command", "stream", "connection"
'ADO objects
If Obj.State <> 0 then
Obj.Close
End If

Case else
'do nothing
End Select

Set Obj = Nothing

Case IsArray(Obj)
Erase Obj

Case Else
'do nothing

End Select

'Now revert it to unitialized state
Obj = Empty
End FunctionUnfortunately I revised Whammy's code in the middle of doing other things (you have no idea how hectic things are at work), and didn't spot his (and now my) faux pas. I have now corrected the code earlier in this thread.

;)

jeskel
11-13-2003, 05:38 PM
how do you exactly use that in your pages M@rco?

M@rco
11-13-2003, 05:48 PM
Set objRegEx = New RegExp
......
......
......
......
Kill objRegExAs you should be able to see from the code it will automatically detect ADO objects and call the .Close() method, and will also destroy arrays correctly. In each case just call it as above, and leave it to do the work!

;)

whammy
11-13-2003, 05:48 PM
I assume like:

Kill(objRegEx)

By the way nice script. Mind if I borrow it? :)

jeskel
11-13-2003, 05:57 PM
wow..... the kind of script tha makes the everyday life easier... mind if use it too? :o

M@rco
11-13-2003, 05:57 PM
You are more than welcome to borrow it - otherwise I wouldn't post it!

;)

And since Kill is a sub (and thus doesn't return a value) you shouldn't use the brackets. However, as you should know, you *can* call a sub with brackets without using the CALL syntax if it has only a single parameter (because it interprets the brackets to mean "the value of"), but nonetheless it's bad practice.

:p

whammy
11-14-2003, 03:14 AM
Yeah I usually refer to subs that way. Brackets are unnecessary if you're not passing args within them. However you can call a sub with no parameters at all and still use brackets. Like "Call blah()".

I'm just curious about your opinion m@rco - it may be a bad practice in VBScript, but I really despise the language since it allows such sloppy coding overall.

I think the majority of the misunderstanding is Microsoft's fault.

There shouldn't be any options regarding calling a subroutine as opposed to a function and/or passing parameters. The language allows very sloppy coding, since it's pretty loose anyway.

That's definitely one of my pet peeves. However, VBScript is an old crappy language and some of us still have to support it. :p

Anyway, my final say is VBScript is sloppy. I'll use brackets because they are appropriate syntax in any cleaner language. I don't really have time at this point to continue best practices in antiquated languages due to bad design. If my way works in .NET and C#, and happens to work just as well in classic ASP, I'm not going to change the way I do things unless I hear a valid argument - which I am very open to. :)

P.S. I am thinking of rewriting that function library for classic asp to close all opened objects.

Kind of a pain in the butt, and to be honest I haven't seen any problems result so I think there might be a local scope to these objects since they are declared within the function, but it needs more testing to make sure that it doesn't eat up server memory.

I would definitely appreciate someone testing the scope and memory consumption of some of these regular expression functions, if they have the time. I'm always open to new ideas and improvements.

M@rco
11-14-2003, 03:28 PM
Originally posted by whammy
Yeah I usually refer to subs that way. Brackets are unnecessary if you're not passing args within them. However you can call a sub with no parameters at all and still use brackets. Like "Call blah()".

I think the majority of the misunderstanding is Microsoft's fault.

There shouldn't be any options regarding calling a subroutine as opposed to a function and/or passing parameters. The language allows very sloppy coding, since it's pretty loose anyway.

I'm just curious about your opinion m@rco - it may be a bad practice in VBScript, but I really despise the language since it allows such sloppy coding overall.

That's definitely one of my pet peeves. However, VBScript is an old crappy language and some of us still have to support it. :p

Anyway, my final say is VBScript is sloppy. I'll use brackets because they are appropriate syntax in any cleaner language. I don't really have time at this point to continue best practices in antiquated languages due to bad design. If my way works in .NET and C#, and happens to work just as well in classic ASP, I'm not going to change the way I do things unless I hear a valid argument - which I am very open to. :)The VBScript syntax *is* rather sloppy, and I suspect that it's because subs and functions are represented in the same way internally. Similarly, VBScript classes are a kludge and not what one would like them to be.

However, I think you are missing an important point - by using brackets to call a sub *without* using the Call syntax, you are introducing inconsistencies in your coding style, since subs with two or more parameters *cannot* be called with brackets. Therefore you must be using two different calling methods depending on how many parameters the sub has, which seems mad to me!!

Incidentally, as I tried to explain earlier (but perhaps didn't do so clearly enough), the reason that you are able to call a sub which has one (or no) parameter WITH brackets is that the brackets aren't interpreted in the same way that a function's brackets are. The brackets are interpreted to mean "the evaluated result of" just as in x = (3+4)*5 the brackets are evaluated (to the numeric value 7) and the result is multiplied by 5. Therefore when you think you are passing a single parameter using brackets, it is actually evaluating the brackets (which is rather pointless since the evaulated value is of course identical to the value you put inside) and then passing the result. I hope that's clear.




Originally posted by whammy
P.S. I am thinking of rewriting that function library for classic asp to close all opened objects.

Kind of a pain in the butt, and to be honest I haven't seen any problems result so I think there might be a local scope to these objects since they are declared within the function, but it needs more testing to make sure that it doesn't eat up server memory.
I would definitely appreciate someone testing the scope and memory consumption of some of these regular expression functions, if they have the time. I'm always open to new ideas and improvements.Pain in the butt?!! All you need to do is add a single line for each object you create in each of your functions. It's hardly going to take you hours, there's no testing required (it won't break anything), and there's no possibility that it will make your code slower (or decrease server performance) - it can only make things *better*.

And besides, making such a basic error in your coding technique can only reflect badly on you if other developers see your code, so it's probably in your own personal best interests to fix it. In fact, if you are posting code here that will copied & pasted by lots of beginners and used as a reference, it is all the more important that the code that you present is as close to "best practice" as you can make it, particularly given your status as forum moderator, from which people will assume that you are an expert on the subject.

I myself have been "ASP Guru of the year" two years' running at Sitepointforums, so I am aware that others follow my advice and use my code, and so I do my best to ensure that my code is as good as possible (best practice, efficient, well-structured, neat presentation), and fix errors/bugs that slip through the net (in code that I have posted) ASAP, and I do the same here as a matter of course.

Caffeine
11-14-2003, 04:51 PM
[confession:begin]
I too call the subs with brackets even though I do not pass any value to it. Always have and always will I guess [always will = short period of time as I'm going to learn C# and asp.Net shortly]

I use brackets when it comes to if-statements too.
[/confession:end]

I do this because I learned Javascript before I learned asp, and I have never really liked the way of doing things in vbscript so at some point I decided that I was going to code as much as I could like 'a real programming-language'.

It may be a bad practice but at the same time I never really planned to do VBScripting for very long [and here I am, almost 3 years later... still coding in VBScript]

M@rco
11-14-2003, 05:26 PM
I use brackets around If tests as well for much the same reasons, but the important difference is that doing so doesn't compromise the consistency of your coding style.

;)

How exactly do YOU call subs which have more than one parameter, WITH brackets, and WITHOUT using the "Call SubName(Param1, Param2)" syntax?

:p



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum