Dangers of using OnChange/ViewState for validation.

Introduction

ASP.NET uses the hidden ViewState field to ensure that each control's OnChange event is only fired when a field is actually changed by a user. So, if a form is submitted three times but a particular field is only changed once, then the OnChange event is only fired once. This can lead to stale validation, application errors, and security flaws.

There are many books and articles on ASP.NET that describe how OnChange works in general, and that web forms can be largely implemented in the same way as client forms. However, I have not seen any that points out the dangers. This article argues that this fundamental feature of ASP.NET should be completely avoided.

The Danger

Suppose, we have an order form that has the following fields:-

Now, suppose that the user submits this form with valid data but invalid shipping details. First, the Order Quantity OnChange event will be checked to ensure that there is stock available. Then the Shipping Details OnChange event will fire, and find the error. The form will be represented to the user for correction. Good so far.

Then the user corrects the shipping details and resubmits the form. The Order Quantity has not changed, so will not be re-validated. If the Shipping Detail is now OK, so the order is accepted.

But what happens if some one else submitted an order which consumed the last of our stock while our user was correcting the shipping details? Our user's order should now fail due to a lack of stock. But the Order Quantity will not be revalidated, so the order will actually be accepted. There is a fair chance that we will end up with a negative stock value stored in the database.

Likewise, the credit limit test introduces a security flaw. Suppose a user has a $1,000 credit limit, then they could create ten sessions and submit 10 orders of $900 each with bad shipping details. Each of them would pass the Credit Limit test before failing the Shipping Details test. The user could then correct the shipping details on all 10 orders and resubmit them. They would all be accepted because the Credit Limit would have already been validated, even though the ten orders are for a total of $9,000.

A worse situation arises if the program calculated the Updated Customer Balance to be, the Current Customer Balance displayed in the form plus the Order Value. Suppose, the user in the previous scenario started with an outstanding balance of $50. Then each of the ten orders would calculate the Updated Customer Balance as $50 + $900. It would do this incorrect calculation 10 times, leaving the balance at $950 even though the user actually ordered a total of $9050 worth of goods. ASP.NET makes it very easy to get at the value of a displayed Label using the ViewState, and so very easy to make this type of mistake.

Discussion

The trouble with errors like these is that they are very hard to test for. The form above would work perfectly well during normal testing. Only in production would strange, irreproducible errors arise. The worst kind.

The fundamental problem is that most validation is being performed against an ever changing database, and yet there is no locking mechanism being used, either pessimistic or optimistic. By allowing the user to change fields during the validation processing, many opportunities for grief are introduced.

Now, it is easy to develop complicated schemes which enable each form to be coded using OnChange and generally be correct. For example, one could store database time stamps with some fields. But in the real world application, correctness should not depend on complicated schemes. I want tools and methodologies that make it very difficult to introduce this type of difficult to reproduce bug, i.e., as idiot proof as possible. (This may say something about the author.)

And for what purpose was OnChange introduced in the first place? To avoid the tiny overhead of having to revalidate fields on those relatively few forms that are submitted with errors? The cost of transmitting the view state over the wire would be much greater.

Client forms need a complex event driven control flow because we want instant, field by field validation. It is an advantage of web forms that validation can be done in a simple single pass, uncluttered by events. This simplifies the logic, and avoids errors. Going out of your way to reintroduce complex event driven control flows seems crazy. This is particularly true when we do not have the stateful ADO locking mechanisms available in web forms that we can use in client forms.

Now it could, of course, be argued that I am just using OnChange for the wrong purpose. Of course, validation should not be done there. But the OnChange/ViewState/WebControl event model seems to be a major feature of ASP.NET. I have seen books and code that use it in exactly this way. It is the default event for Visual Studio when double clicking on a control. And if it was only designed for more obscure uses, then why is ViewState enabled by default? I'm sure that many programmers would be confused.

Recommendation

Never use OnChange. Just do the validation during page load. Repeat validations, if necessary, each time a form is submitted, to avoid potential bugs. Keep it simple, keep it fast, keep it correct.

This also suggests that you always suppress the ViewState, as you will not be using it. No potential encryption security flaws to worry about.

Feedback

I am hoping to start a lively discussion about what is really the best way to use ASP.NET to build simple, robust applications. I would also invite comments on other ASP.NET gotchas that have received little attention.

It amazes me that I have not seen this fundamental issue discussed before, which has inspired this article.