views:

101

answers:

9

I'm building a RoR site, and today I get the pagination done. Upon showing it to my coworker, his first question is "what happens if you set the querystring to "?page=-1". It died with a runtime exception (error 500). He suggested that that should definitely be fixed before this site goes anywhere near live.

I happen to disagree with him (hear me out). Now, I've been in the web dev business for all of four months, so I very well could be wrong. But I would think that this isn't a big deal. I would think that, so long as said errors do not constitute a security risk, things like this shouldn't be a priority. The only way to cause this error is if you manually edit the query string, and, well, garbage in garbage out. If you're smart enough to know that you even can edit the querystring, you should be smart enough to not give it a negative number.

What is the general consensus on things like this? Do you completely idiot proof the site, so that no matter what the query string is, you never generate an error? Do you let things slide so long as it works the way it's supposed to (and doesn't expose a security risk)? Somewhere in the middle?

EDIT: Somehow my question didn't really come out completely as I intended it. The crux of my question was, where to draw the line between proactively correcting for things versus not doing them. If there's invalid input in the get string, for instance, would it be better practice to display a tasteful error as suggested in the posted replies, or to try to figure out what the user was doing, and do that. Or, as a more concrete example: If a user sets page=-1 in the get string, would it be better to silently assume they meant page=0, or to display some kind of tasteful error page saying somethign like "invalid page specified"?

+2  A: 

If you have an error-page that looks nice, and gives a polite message, I'd say it's fine. Though I might consider responding with a 404 instead. Garbage in should preferably not produce an error.

aioobe
+3  A: 

You make some good points, but an incorrect query string can have many reasons. For example, a link to a record that has since been deleted. Or a Google result pointing to a page that doesn't exist in the current result set any more.

In these cases, you should show the user something a bit more verbose than a 500 error.

Pekka
+3  A: 

At the very least you should come up with a nicer-looking page than a standard 500 error page. So yes, you would have to trap errors and handle them.

CanSpice
A: 

I agree with you, that error messages are necessary and useful but you should try to differentiate, e.g. give an 404 where the user requested a page that doesn't exist.

levu
A: 

erauqssidlroweht - i don't know what your develpoment platform is. but in asp.net mvc, you can trap such inconsistancies in the controller and present a custom error page to the user.

given that you're only 4 months 'old' at this :), i'd consider it a priority to address such things to enable you to deal with the inner workings of the potential problem areas in any context.

just my thoughts...

jim
he did say "RoR"
Adriano Varoli Piazza
... oops, you're correct. age and eyesight, combined with impatience :)
jim
+7  A: 

You should be error checking anything that comes in from the query string. If you get an invalid page number, you should have an error message that's a little more graceful than the Error 500 page. Maybe a sorry, bad request. Try this: <possible suggestions>. It's just plain sloppy and unprofessional to knowingly and deliberately leave an easily accessible error like that on a live site.

You say you're new to web apps, but if your previous dev experience was other GUI apps being used by the "general public" (non-developers, non-techies), would it have been OK to have stack traces thrown into the user's face as the app falls apart around them? In my experience, this is never really acceptable.

FrustratedWithFormsDesigner
+1. I don't think you could ever make your site/application idiot proof (idiots are pretty smart these days), but this is definitely the minimum.
TandemAdam
@TandemAdam: No, you're right, as soon as you think it's idiot proof, someone builds a better idiot! But yeah, there are certain minimums that can be maintained. Especially if a client's paying for this. If I paid someone to build my site, this would definitely by on my test list and the "Error 500" result would be a definite no-go.
FrustratedWithFormsDesigner
Unfortunately my previous work experience was "university". This is the first real project I've worked on. And in university, we never touched GUIs except for my one Software Engineering course I took in my last term. That being said, you're right. Definitely not ok to throw stack traces in the user's face.
erauqssidlroweht
+1  A: 

I don't think a 500 error page is very meaningful to your average user. At least tell him something is wrong with your page and guide him back on the right track by providing a link to get back to your site.

Sometimes I redirect users to a page that is likely to what he wanted. So when a query goes below zero and this is not permitted, redirect your user to ?page=0 and maybe display a message on top of that page. I think you should prefer this method because it is a better approach in terms of user experience to not use modal windows.

Jan
This is what I'm now doing. Any invalid page value defaults to page=1 (first page/most recent posts)
erauqssidlroweht
I think a notice to the user is appropriate, just don't make it a modal dialog or navigating to an error page when nothing critical happens. (in my opinion, supplying wrong query parameters is non-critical)
Jan
A: 

It varies from project to project. How many users do you expect? If it's below 10K visitors a day it might not be so bad. What percentage of users do you expect will hit the problem? I don't expect that very many but you would know best.

The goal should be to ship the product and roll out improvements regularly. Hopefully the product is sound overall.

Regarding a solution, if its a page not found, a 4xx error should be thrown instead of a 5xx. 5xx errors typically warrant a deeper look and while it's hard to write an air-tight application directly on launch, you should try to have a generic handler for 4xx and 5xx errors.

aleemb
A: 

In the PCI game (Credit Card Verification / Validation) the rule is validate everything and allow for no idiots. So the answer depends on your application.

Dave