Doug asks:
Hey Ray, do you think sometime you could discuss using cfqueryparam versus the need to have "elegant" ways of handling bad data? It seems like a better solution would be to redirect the user back to a default page than to have a page crash because of bad data or a hacking attempt.
I guess my point is: isn't this code:
<cfif isNumeric(url.this)> <cfquery name="theQuery" datasource="ds"> SELECT this, that FROM theTable WHERE this = url.this </cfquery> <cfelse> <cflocation url="/SomePlaceSafe.cfm"> </cfif>
more elegant than this code:
<cfquery name="theQuery" datasource="#ds#"> SELECT this, that FROM theTable WHERE this = <cfqueryparam value="url.this" cfsqltype="cf_sql_integer" /> </cfquery>I've seen you complain about this before in your Twitter feed, so I figured you had an opinion about it. (And for the sake of argument let's ignore claims that cfqueryparam actually improves performance with certain databases.)
Interesting question, Doug. I'll try my best to answer it. First off, I think you make a mistake when you say that 'elegant' handling of the error is impossible with cfqueryparam. It certainly isn't. In fact, one could simply take your code examples and merge them:
<cfif isNumeric(url.this)>
<cfquery
name="theQuery" datasource="#ds#">
SELECT this, that FROM theTable WHERE this =
<cfqueryparam value="#url.this#" cfsqltype="cf_sql_integer" />
</cfquery>
<cfelse>
<cflocation
url="/SomePlaceSafe.cfm">
</cfif>
And in fact, this is actually exactly what I do. Well, normally I'm writing in Model-Glue and instead do the validation at the controller layer and the query in the model layer, but I think you get my point. You certainly can actually double up on the validation here. It may be overkill, but when it comes to URL parameters and other things coming in from the remote client, you can't be too careful. (We all learned this last week with the FCK Editor issue, right?) Also note that both of those checks in the code above even aren't truly enough. 3.14159 is a numeric value, right? But I'm pretty sure that will either fail in the cfqueryparam or change to 3. -3 is also an integer value, but if you are using autonumber keys then you will not have any negative values. I'll often do something like this:
<cfif structKeyExists(url,"this") and isNumeric(url.this) and url.this gts 1 and round(url.this) eq url.this>
And if you want to get truly evil, you could also check for values greater than MAXINT (the largest number the underlying Java code can handle before flipping over).
So where exactly to use validation is a big issue. As I said, I'm normally going to do it in the controller. I've done it in beans before as well. Frankly, I'm still learning what works best for me, but the main point is, you can get both the security/performance benefit of cfqueryparam and still gracefully handle input validation yourself as well. Shoot, even if you are lazy you could get by with an onError in your Application.cfc and notice all errors thrown by queryparam and log them as (possible) url hack attempts before pushing the user to the safe place.
Archived Comments
Hi Ray and Doug:
I am a huge proponent of cfqueryparam only in this scenario. I believe the best approach here is to try catch the query and either bubble or handle accordingly with the cflocation. I wrap all 3rd party calls; db, file, com, etc with try catch as they are ultimately outside the the cf engine and can produce unexpected results (for instance if the db is down or unstable).
It is also much less code and validation is really an after thought. I believe the input (text box in html or flex) should be validated client side before ever going to the server as well.
Hope this code comes out in the comment:
<cftry>
<cfquery name="theQuery" datasource="#ds#">
SELECT this,
that
FROM theTable
WHERE this = <cfqueryparam value="#url.this#" cfsqltype="cf_sql_integer" />
</cfquery>
<cfcatch type="database">
<!--- Either bubble with cferror and pass on to global handler, i.e. onError in app.cfc
or just place your cflocation in the catch (used in this example --->
<cflocation url="/SomePlaceSafe.cfm">
</cfcatch>
</cftry>
My issue with this is that there is no 'pretty' way to represent the error. Ie, the normal error from cfqueryparam would mean nothing to the end user. However - if you are using client side validation to handle that, then I think you are ok. The combination of client/server validation with the client side being 'elegant' and the server being 'brute' should be acceptable.
Jeff's solutions seems much better in the long run. What happens when the validation rules change? You'd have to go back and tweak all the if/else around each query.
And hopefully everyone is using the jQuery validation plugin :)
I think the really problem here is that your trying to combine to separate tasks in to one. A create or update method should just expect that the data its going to receive is going to be good data. The only thing I catch an exception for at that point is a database connection error. The validation should be pulled out and be performed before any save action is taken. A usual case for me is:
1.) populate my object with data
2.) validate
3.) if object has errors display them
else save
Interesting. I usually do my validation in the bean (I almost always use beans to pass data to a function if there are more than one or two arguments to the function...just makes it a bit tidier). I guess my philosophy is that if the bean is reponsible for carrying my data around, then it ought to know what it's carrying and if it is valid for the purpose intended. The bean's validate method returns a boolean value and I test this before ever getting to the object containing the db query. As noted, db exception messages are scary to the end user and tell them nothing.
@Jim Priest
Jim, if you're subscribing to this post, what's the "jQuery validation plugin"..?
I am curious to know why everyone decided to do validation at the beat level? This is by no means a stab at Joel but more of a question to all. Did everyone start doing validation at the bean level because it made sense or because they had nowhere else to do it? A class (component in this case) should do 1 task and do it well. A bean accomplishes 1 task very well and that is representing a single object. I think validation should be done on it's own terms and if that means passing in data or passing in bean I am fine with either. Just my 2c!
@Jay
Check out Jorn's plugin here: http://bassistance.de/jquer...
Do we really care if hackers get an elegant error message instead of a default error page?
@JC,
Oh, absolutely. If the exception message gives details about the database system, table names, column names, etc, this is valuable information for further attacks.
To follow up on Jim Priet's URL, also note I did about 4 blog entries here on that plugin alone. Just browse my jQuery category.
@JC - also if you generate email reports of your errors to review (which is always a good idea), I'd much rather give a hacker an online error message than one that I have to look at.
I am not a big fan of causing the database to throw errors, even unintentionally. If the content of a variable passed to the database is going to cause a DB error, it seems the better approach to protect the DB by prior validation.
Defense in depth should be the order of the day. A nice looking default error message is good, but preventing information leakage is better. User input is pure evil until you prove otherwise.
Applying javascript error handlers using ColdFusion.Ajax.submitForm has kind of changed the way I look at this type of issue over the last 2 years. I know that it is a lot of extra work to some folks but you can flat eliminate passing invalid data to your db if you put in the work up front.
My approach has changed quite a bit and I find myself often using cfajaxproxy along with front end javascript validation a lot more than serverside error handling techniques. I'm consistent in using cfqueryparam on the back end but trapping crap data up front and stopping the process before the submit even happens is really working out for me.
It is a lot of extra work but using cfajaxproxy to check for primary key violations in instances where the user names a key is helpful (subsidiary tables that employ meaningful user defined codes rather than manufactured primary key values to make records unique). For instance, nobody wants a unit of measure code named 2, they want lbs or units.
Just my opinion.
Mary Jo hit it spot on. I get emails from the onError method, but fact of the matter is, I don't want to get emailed for hundreds of failed attempts to inject something into my DB. So, instead of my website's standard "we're sorry you got this error" message, I'd rather just send the user back to the main page of the section they're in. This way has the advantage that if it was bad url data passed for some innocent reason (such as mistyping a link), the person won't have to see the error message at all.
Thanks for the tips Ray! (And I've gotta put my fear aside and start dabbling in ModelGlue. Time to read your next blog post...)
from what i am reading about objects and oop i reason that data validation should take place where the data belongs. A bean contains data, it is it single responsibility to hold the (correct) data and present it to other parts of the application. So i put validation into the bean and by doing so i also encapsulate the bean from the rest of the application. The other parts of the application do not have to worry about the correctness of beans data, they just assume that it is correct.
Take a look here: http://www.martinfowler.com...
of course cfqueryparam and cftry, cfcatch should always be part of the object that access the database
@Ray, no way would I throw native back to the client! 8-)
Though, I'm concerned about some of the responses here. Who says the db throws a native error in my scenario @Dave Crawford? It never gets to the db if the cfqueryparam fails.
As for custom error handling that is a given. I eluded to bubble to a global handler; CF 101 or app dev in general, never show or present native errors to the user. I usually roll my own error handler .cfc which handles the error per the given process (make the error as pretty as you like here) i.e. Send an email, custom log with cflog, etc.
There you can present a more intelligent response to the user so that customer support can do their job effectively. This kind of information may go as far as including template, line number, and query name info. Alot of flexibility here or just cflocation afterward per Doug's initial example.
As for the validation discussion, whether you incorporate that client or server side is personal choice. I don't like taxing the server if I can do it client side (remember the old cfform js trick?). This goes years back, but I would have the server gen the js for me and switch back to html form. Many advances with jQuery here agreeed... I have worked on projects where it is done both client and server. I personally have no issue with implementing in the Bean per @Martin's response.
@Andy, sorry just reread your reply. Totally agree with your approach.
Using a form I'll validate with CFFORM using both onSubmit and onServer, that way I know the data is validated by the time it gets there. If I need to do some extra post-submittal validation (like checking to see if a record already exists before allowing an insert) I'll write it up server side.
Ultimately I don't want to rely on CFQUERYPARAM to catch data type failures, it's the last line of defense for me. I'll cover every variable in a query, but I'm more interested in the performance boosts CFQUERYPARAM provides. The added security is an extra benefit to me, since I should have covered the security needs beforehand.
I guess that's what it boils down to, right? CFQUERYPARAM is just the last line of defense, which we SHOULD use, but only after all the "elegant" methods are exhausted, (such as client-side and server-side validation, redirecting bad url, form, and cgi submissions, custom error handling, etc.)