Invalid form input leads to broken SQL

In some cases, invalid input to exposed elements of the view leads to broken SQL, and an ugly error message. This is also _falsely_ triggered by some security scanners as a SQL injection vulnerability.

The specific case I've seen is for a page view with an exposed operator, when an illegal value is passed in the operator parameter. Here's the code which I think is the root of the issue (in view.inc, function build($display_id = NULL)):


if ($this->display_handler->uses_exposed()) {
$this->exposed_widgets = $this->render_exposed_form();
if (form_set_error() || !empty($this->build_info['abort'])) {
$this->built = TRUE;
return empty($this->build_info['fail']);
}
}


When form_set_error() to return errors, but build_info['abort'] and build_info['fail'] are empty, the return value from the build() function is TRUE but the actual query is never built. You end up with a message like this:

# An illegal choice has been detected. Please contact the site administrator.
# user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'INNER JOIN node_access na ON na.nid = node.nid WHERE (na.grant_view >= 1 AND ((n' at line 1 query: SELECT COUNT(*) FROM ( INNER JOIN node_access na ON na.nid = node.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 0 AND na.realm = 'og_public') OR (na.gid = 1 AND na.realm = 'term_access')))) count_alias in .../sites/all/modules/views/includes/view.inc on line 760.

As you can see, it's trying to select count(*) from an broken subquery.

... SELECT COUNT(*) FROM ( INNER JOIN node_access ...

My first thought was to just put a patch in the set build_info['fail'] to TRUE whenever form_set_error() is true, but then I was afraid that this would break some cases where errors are correctly handled.

Here's what I've got now, but it's ugly as sin.

if ($this->display_handler->uses_exposed()) {
$this->exposed_widgets = $this->render_exposed_form();
if (form_set_error() || !empty($this->build_info['abort'])) {
if (form_set_error()) {
foreach (form_set_error() as $key => $value) {
if ($value == t('An illegal choice has been detected. Please contact the site administrator.')) {
$this->build_info['fail'] = TRUE;
}
}
}
$this->built = TRUE;
return empty($this->build_info['fail']);
}
}


Wold someone who knows the views module let me know what they think?

Also, I believe this is what comment #5 in http://drupal.org/node/370680 is about, and also what http://drupal.org/node/631640 is about, but I'm not confident enough to go monkeying around and marking things as duplicates.

Thanks!