Showing posts with label bug. Show all posts
Showing posts with label bug. Show all posts

ASP.Net MVC Unobtrusive Validation Bug

If you use the ASP.Net MVC 3 [Compare] validation attribute on a model property, then include that model as a property in a parent model (so that the field name becomes Parent.ChildProperty), the built-in unobtrusive client validation will choke, and will always report the field as having an error.

This is due to a bug on line 288 of jquery.validate.unobtrusive.js:

adapters.add("equalto", ["other"], function (options) {
    var prefix = getModelPrefix(options.element.name),
        other = options.params.other,
        fullOtherName = appendModelPrefix(other, prefix),
        element = $(options.form).find(":input[name=" + fullOtherName + "]")[0];

    setValidationValues(options, "equalTo", element);
});

Because the value of the name attribute selector is not quoted, this fails if the name contains a ..

The simplest fix is to add quotes around the concatenated value.  However, the jQuery selector there is overkill.  HTML form elements have properties for each named input, so you can do this instead:

adapters.add("equalto", ["other"], function (options) {
    var prefix = getModelPrefix(options.element.name),
        other = options.params.other,
        fullOtherName = appendModelPrefix(other, prefix),
        element = options.form[fullOtherName];
    if (!element)
        throw new Error(fullOtherName + " not found");
    //If there are multiple inputs with that name, get the first one
    if (element.length && element[0])        
        element = element[0];
    setValidationValues(options, "equalTo", element);
});

The Dark Side of Covariance

What’s wrong with the following code?

var names = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
...
if (names.Contains(sqlCommand.ExecuteScalar())

This  code is intended to check whether the result of a SQL query is contained in a case-insensitive collection of names.  However, if you run this code, the resulting check will be case-sensitive.  Why?

As you may have guessed from the title, this is caused by covariance.  In fact, this code will not compile at all against .Net 3.5. 

The problem is that ExecuteScalar() returns object, not string.  Therefore, it doesn’t call HashSet<string>.Contains(string), which is what it’s intending to call (and which uses the HashSet’s comparer).  Instead, on .Net 4.0, this calls the  Enumerable.Contains<object>(IEnumerable<object>, string) extension method, using the covariant conversion from IEnumerable<string> to IEnumerable<object>.  Covariance allows us to pass object to the Contains method of any strongly-typed collection (of reference types).

Still, why is it case-sensitive?  As Jon Skeet points out, the LINQ Contains() method is supposed to call any built-in Contains() method from ICollection<T>, so it should still use the HashSet’s case-insensitive Contains().

The reason is that although HashSet<String> implements ICollection<string>, it does not implement ICollection<object>.  Since we’re calling Enumerable.Contains<object>, it checks whether the sequence implements ICollection<object>, which it doesn’t.  (ICollection<T> is not covariant, since it allows write access)

Fortunately, there’s a simple fix: just cast the return value back to string (and add a comment explaining the problem).  This allows the compiler to call HashSet<string>.Contains(string), as was originally intended.

//Call HashSet<string>.Contains(string), not the
//covariant Enumerable.Contains(IEnumerable<object>, object)
//http://blog.slaks.net/2011/12/dark-side-of-covariance.html
if (names.Contains((string)sqlCommand.ExecuteScalar())
(I discovered this issue in my StringListConstraint for ASP.Net MVC)

Beware of Response.RedirectToRoute in MVC 3.0

ASP.Net MVC uses the new (to ASP.Net 3.5) Http*Base wrapper classes (HttpContextBase, HttpRequestBase, HttpResponseBase, etc) instead of the original Http* classes.  This allows you to create mock implementations that inherit the Http*Base classes without an actual HTTP request.  This is useful for unit testing, and for overriding standard behaviors (such as route checking).

In ordinary MVC code, the HttpContext, Request, and Response properties will return Http*Wrapper instances that directly wrap the original Http* classes (eg, HttpContextWrapper, which wraps HttpContext).  Most MVC developers use the HttpContext and related properties without being aware of any of this redirection.

Until you call Response.RedirectToRoute.  This method, which is new to .Net 4.0, redirects the browser to a URL for a route in the new ASP.Net routing engine.  Like other HttpResponse methods, HttpResponseBase has its own version of this method for derived classes to override.

However, in .Net 4.0, Microsoft forgot to override this method in the standard HttpResponseWrapper.  Therefore, if you call Response.RedirectToRoute in an MVC application (where Response is actually an HttpResponseWrapper), you’ll get a NotImplementedException.

You can see this oversight in the methods list for HttpResponseWrapper.  Every method except for RedirectToRoute and RedirectToRoutePermanent are list as (Overrides HttpResponseBase.MethodName().); these methods are listed as (Inherited from HttpResponseBase.MethodName().)

To work around this issue, you can either use the original HttpResponse by writing HttpContext.Current.Response.RedirectToRoute(…) or by calling Response.Redirect instead.

Note that most MVC applications should not call Response.Redirect or Response.RedirectToRoute at all; instead, they should return ActionResults by calling helper methods like return Redirect(…); or return RedirectToAction(…);

In the upcoming ASP.Net 4.5 release, these methods have been properly overridden.

XRegExp breaks jQuery Animations

XRegExp is an open source JavaScript library that provides an augmented, extensible, cross-browser implementation of regular expressions, including support for additional syntax, flags, and methods.

It’s used by the popular SyntaxHighlighter script, which is in turn used by many websites (including this blog) to display syntax-highlighted source code on the client.  Thus, XRegExp has a rather wide usage base.

However, XRegExp conflicts with jQuery.  In IE, any page that includes XRegExp and runs a numeric jQuery animation will result in “TypeError: Object doesn't support this property or method”.

Demo (only fails in IE)

This bug is caused by an XRegExp fix for an IE bug in which the exec method doesn't consistently return undefined for nonparticipating capturing groups.  The fix, on line 271, assumes that the parameter passed to exec is a string.  This behavior violates the ECMAScript standard (section 15.10.6.2), which states that the parameter to exec should be converted to a string before proceeding.

jQuery relies on this behavior in its animate method, which parses a number using a regex to get the decimal portion.  (source)

Thus, calling animate with a number after loading XRegExp will fail in IE when XRegExp tries to call slice on a number.

Fortunately, it is very simple to fix XRegExp to convert its argument to a string first:

if (XRegExp) {
    var xExec = RegExp.prototype.exec;
    RegExp.prototype.exec = function(str) {
        if (!str.slice) 
            str = String(str);
        return xExec.call(this, str);
    };
}
Here is an updated demo that uses this fix and works even in IE.