#limit_validation_errors fails is parents array contains numeric indexes

Good catch. This should work:

array_slice(explode('][', $name), 0, count($section)) == array_values($section)

I guess we need tests here.

Definitely "major" since field items have numeric indexes.

Actually, no! Because 0 == 'foo' is true. I guess rather than exploding $name, you can implode $section, and then compare as strings.

@yched: Sorry I've delayed so long on this. I was hoping to get some time to look at this in more detail, and maybe even help with a new patch, but since that hasn't happened, and since we're nearing RC, here's a cursory review. I think this issue is critical, since it may lead to a security vulnerability, though I haven't looked at it closely enough to know for sure if the existing bug can be exploited as a security hole.

+++ includes/form.inc
@@ -1423,12 +1423,15 @@ function form_set_error($name = NULL, $message = '', $limit_validation_errors =
+        $section_string = implode('][', $section);
+        if (strpos($name, $section_string) === 0) {

I think this should probably be changed to strpos($name . ']', $section_string . ']') or something like that, so that 'foobar' isn't considered "inside" 'foo'. We need a test for this as well, both at the top level, and a nested level, like 'parent][foo'.

+++ modules/simpletest/tests/form.test
@@ -461,16 +461,24 @@ class FormValidationTestCase extends DrupalWebTestCase {
+    // Edge case of #limit_validation_errors containing numeric indexes: same
+    // thing with the 'Partial validate (numeric index)' button and the
+    // 'test_numeric_index' field.

Is it really an edge case, if it happens within Field API?

Powered by Dreditor.

Added the suggestion from @effulgentsia in #6 to prevent substrings from matching incorrectly. Also added a test for it.

Marking as needs review to trigger test.

This is the same patch as above but the change is the smallest possible to fix the problem. I like small changes, you know?

In case of a reroll, patch contains trailing whitespace.

Discussing with webchick, actually we can throw out array_slice and just limit explode.

@tsoeckler , mine? The previous contained fixing whitespace which I skipped.

Ok, committed to HEAD!

Unfortunately this patch has totally broken top level validation with #limit_validation_errors, e.g.:

$form['test'] = array('#tree' => true);
$form['test']['inner'] = array(
  '#title' => t('Should be validated'),
  '#type' => 'textfield',
  '#required' => TRUE,
 );

$form['actions']['submit'] = array(
  '#type' => 'submit',
  '#value' => t('Should validate test and inner'),
  '#limit_validation_errors' => array(array('test')),
  '#submit' => array('some_submit_function'),
);

With the patch applied in #11, $form_state['values']['test']['inner'] is never validated, without the patch it is validated.

patch in #9 works and successfully validates children.

Committed to CVS HEAD. Thanks.