views:

165

answers:

5

Hello all,

Consider the following scenario: http://www.yourdomain.com/Default.aspx?p=2

Now we ofcourse want to check if the querystring parameter p doesnt contain errors. I now have this setup:

1) Check if p exists

2) Filter out html from p's value

3) htmlencode p's value

4) check if p is integer

5) check if p's integer exists in db

This is how I usual do it, though step 5 is ofcourse a performance hit.

Kind regards, Mark

+3  A: 

If your intent is to use the parameter to retrieve something from the database, why filter out html or encode it? It's not like you're going to store it in the database, or display it on the front end. Just immediately throw it to the DAL if it exists. You're DAL should be smart enough to tell you if it failed to retrieve a record with that ID, or if the ID couldn't be parsed, etc..

Joseph
+3  A: 

My view: Generally a querystring parameter of this kind isn't really "entered" by users but is submitted as a link. So over-complex slow validation isn't really necessary.

So I would just pass this through to the persistence / data layer and handle any errors that come back as a regular 404 Not Found or 500 Internal Server Error depending on the kind of system I'm working with.

Jeremy McGee
Agreed. I tend to just check it isnt null on the page / control then pass it through the layers and check here.
Jammin
Or why not just let exception handling do its thing? This is what it's there for.
Jeremy McGee
+1  A: 

If you are going to convert the input to an integer anyway, then steps 2 and 3 are not needed - just use int.TryParse to see what you have. I would encode and test the input for html only if you are expecting a string which you will use in a dynamic sql statement, or will be displaying on your site

Ray
+1  A: 

What about:

int p = 0;
if(!Int32.TryParse(Request.QueryString["p"], out p))
    throw new ArgumentOutOfRangeException("p");
Rubens Farias
A: 

Quite simple. For most data types (integers, decimals, doubles, dates and booleans) there is a very strict format. If the value does not parse under the strict format, it's an error.

Strings sometimes have a strict format, like an email address or a phone number. Those can be validated with a simple regexp. If it conforms, use it, otherwise it's an error.

Most of the time however strings will simply need to be persisted to the DB and later displayed again. In that case no processing is needed, aside from escaping when inserting into DB (unnecessary as well if you used parametrized queries)k, and HTML-encoding when rendering to the display.

This way any and all data is validated, and there is no risk of any injections whatsoever.

The rare exception of a loose format for a string is, well... rare. I can't think of any right now. For that you can afford some more extensive parsing and processing.

Added: Oh, yes, checking whether IDs (or other values) are valid in respect to a DB. You're doing it right, but think if you always need it. Quite often you can put the check into some other query that you have to do anyway. Like when you select data based on the ID, you don't need to explicitly check that it exists - just be ready that your query can return no data.

Sometimes you don't need to use the value at all, then you can simply ignore it.

But, of course, there are other times, like when inserting/updating data, that you indeed need to explicitly check whether the data exists and is valid in the current context.

Vilx-