form_select_options() causes infinite recursion in the installer when it's trying to generate markup for the options that are objects.

I'm not sure when in the past #type => 'select' #options were objects, but now, sometimes this function is getting an $element where the #options are all Twig_Markup objects.

Here's form_select_options() from core/includes/form.inc:

function form_select_options($element, $choices = NULL) {
  if (!isset($choices)) {
    $choices = $element['#options'];
  }
  // array_key_exists() accommodates the rare event where $element['#value'] is NULL.
  // isset() fails in this situation.
  $value_valid = isset($element['#value']) || array_key_exists('#value', $element);
  $value_is_array = $value_valid && is_array($element['#value']);
  $options = '';
  foreach ($choices as $key => $choice) {
    if (is_array($choice)) {
      $options .= '<optgroup label="' . $key . '">';
      $options .= form_select_options($element, $choice);
      $options .= '</optgroup>';
    }
    elseif (is_object($choice)) {
      $options .= form_select_options($element, $choice->option);
    }
    else {
      $key = (string) $key;
      if ($value_valid && (!$value_is_array && (string) $element['#value'] === $key || ($value_is_array && in_array($key, $element['#value'])))) {
        $selected = ' selected="selected"';
      }
      else {
        $selected = '';
      }
      $options .= '<option value="' . check_plain($key) . '"' . $selected . '>' . check_plain($choice) . '</option>';
    }
  }
  return $options;
}

When a $choice is a Twig_Markup object, this line is totally wrong:

     $options .= form_select_options($element, $choice->option); 

Because Twig_Markup doesn't have an 'option' element. (This line is the same way in D7... what is ->option for?) This causes the recursion to happen with $choice being passed as NULL, which causes infinite recursion.

Comments

adamdicarlo’s picture

Title: form_select_options() crashes installer when encountering Twig_Markup objects » form_select_options() crashes when encountering Twig_Markup objects
Status: Active » Needs review
StatusFileSize
new2.35 KB

I wrote a patch that I believe fixes the problem.

I couldn't find anywhere in core that uses the object-with-an-array-inside for SELECTs, so I've ripped out support for that.

fabianx’s picture

Status: Needs review » Closed (won't fix)

Hi,

Thank you very very much on your work on the twig_engine branch.

However I have to close this, because the branch is deprecated - now that twig officially is in core.

I was not able to sync it all up within all versions always and the fix for the above ended up to be only in some sub branch in my performance testing sandbox.

We ended up with a different approach in #1818266: [meta] A secure theme system (with twig), because #1712444-16: Twig: Activate autoescape mode was unable to pass tests.

So thank you again for your work on this. The sandbox should be in a better state to work with next week.

adamdicarlo’s picture

Hmm, I had started to wonder why I was told to work on the twig_engine branch at the code sprint. Once I wrote this patch the installer worked but pretty much every admin page was crashing.

Ah well. I'm back from BADCamp but I may have some time to help out more soon. I'll keep an eye on the sandbox. Thanks for the reply.

Project: » Lost & found issues

This issue’s project has disappeared. Most likely, it was a sandbox project, which can be deleted by its maintainer. See the Lost & found issues project page for more details. (The missing project ID was 1750250)