Escaped checkbox option values are not saved (keys with quotes or ampersands)

The quick solution for this is to use a key with alphanumeric characters like this:

rocky_bullwinkle|Rocky & Bullwinkle
moose_squirrel|Moose and Squirrel

Though I think you're right, all characters should be allowed or at least we should add validation to prevent problems with keys.

This is a tricky problem. If we don't escape the value of checkboxes, then users can enter single or double quotes and cause the HTML rendering to break. According to HTML spec, the contents of a "value" attribute should be HTML encoded before displaying. So Webform's escaping is technically correct, but Drupal can't keep track of the submitted values correctly.

A good temporary solution is just to display the select component as a list, which doesn't have all the bugs of checkboxes (which are notoriously difficult to deal with). I fiddled with trying to fix this problem but didn't have much success.

Subscribing.

This problem still exists in the latest 4.x version as well. Using an ampersand in the key value of a checkbox throws the "An illegal choice has been detected. Please contact the site administrator." error message.

See additional commentary in duplicate issue:
#1677880: Using special html entites like &, quote or double quote etc. as key for options of webform select component.

Testing the current implementation (7.x-4.3):

TEST CASES
blue & red|Blue & Red
a > b|a > b
Dan's|Dan's
"no way"|"no way"
<script>alert("hi");</script>|<script>alert("hi");</script>
<strong>stronger</strong>|<strong>stronger</strong>

LISTBOX / MULTIPLE
Displayed in form exactly as human values.
Displayed in submission as safe HTML: a > b, alert("hi"); stronger (in bold)
Saved in database as exact machine values.
Verdict: Acceptable. Better if form matched submissions.

LISTBOX / SINGLE
Same as multple

NOT LISTBOX / MULTIPLE = CHECKBOXES
Displayed in form as safe HTML (same as LISTBOX submission). Form cannot be submitted.
Verdict: Failure

NOT LISTBOX / SINGLE = RADIO BUTTONS
Displayed in form as safe HTML (same as LISTBOX submission).
Displayed in submission same as form.
Saved in database as exact machine values.
Verdict: Perfect

Edit: Hmmm. The code tags mess with line spacing in a very odd way. Oh well.

Researching this. Webform currently contains the following replacement for expanding checkboxes. It installs this in lieu of form_process_checkboxes().

function webform_expand_checkboxes($element) {
  // Elements that have a value set are already in the form structure cause
  // them not to be written when the expand_checkboxes function is called.
  $default_value = array();
  foreach (element_children($element) as $key) {
    if (isset($element[$key]['#default_value'])) {
      $default_value[$key] = $element[$key]['#default_value'];
      unset($element[$key]);
    }
  }

  $element = form_process_checkboxes($element);

  // Escape the values of checkboxes.
  foreach (element_children($element) as $key) {
    $element[$key]['#return_value'] = check_plain($element[$key]['#return_value']);
    $element[$key]['#name'] = $element['#name'] . '[' . $element[$key]['#return_value'] . ']';
  }

  foreach ($default_value as $key => $val) {
    $element[$key]['#default_value'] = $val;
  }

  return $element;
}

The D6 issue where the code to remove and re-created the defaulted checkboxes:
#243839: Page Break not working in Drupal 6

The D6 issue where the escaping was added:
#354240: Answers in select fields not accepted if questions contain apostrophe AND the field accepts multiple answers

The D7 issue where the #process function is rather carefully installed to avoid conflicts with other #process functions:
#1188774: Select compoent should not directly set '#process' option for select_or_other

In my testing, this process function is entirely not needed. The escaping results in double-escaping (and hence the bug reported here). The key is placed in the 'value' attribute of the input element, and is escaped with check_plain in drupal_attributes. The value is placed in #return_value, which gets turned into the value attribute and is also escaped above.

I can't reproduce any issue with these elements already being in the form for some reason. There is a comment implying an issue with multi-page forms, but I could not reproduce any such issue. I suspect this is related to some D6 issue which is no longer needed in D7.

I've uploaded a patch which simply removes the above process function. I would like some thorough testing of difficult checkbox keys and values, and editing existing already-checked submissions. Difficult keys should include keys that need escaping, such as & and attempts to introduce XSS vulnerabilities, such as:

a<script>alert('key');</script>b|There should be no alert <script>alert('value');</script>

Committed to 7.x-4.x and 8.x

I've cherry-picked this into 7.x-3.x and it will be part of the 7.x-3.23 release.

There was a merge conflict so I'm also attaching the patch.