HTML entities aren't handled correctly when adding a product reference

the title is getting double escaped in commerce_product_reference_options_list() since sku and title get escaped already in commerce_product_match_products() via _commerce_product_match_products_standard().

The attached patch removes the escaping in commerce_product_reference_options_list().
The patch is a -p0 patch.

Patch applied, fixed my display issues.

Hmm, actually, they get sanitized through the "rendered" key in the return value, but the individual values themselves aren't sanitized that I see. I could be missing it given the late hour, of course. I wonder if instead it's the options list function that invokes this callback performing the duplicate sanitization... will have to look into it tomorrow.

Applied the patch, but it didn't work for me. I am running the latest dev version of drupal commerce.

Edit: When adding a product to the product reference field in the edit form it displays properly. However, the entities are not escaped when actually looking at the product.

As I mentioned before my problem was actually that html entities were not displaying properly in the add to cart form.

I believe I have found the problem. Here is my fix for it.

commerce_cart.module line 1469

  function commerce_cart_add_to_cart_form
...
  // If the products referenced were of different types or did not posess
      // any qualifying attribute fields, add a product selection widget.
      if (!$same_type || empty($qualifying_fields)) {
        $options = array();

        foreach ($products as $product_id => $product) {
          $options[$product_id] = htmlspecialchars_decode(check_plain($product->title),ENT_QUOTES);
        }

Please check into this to make sure everything is working properly.

The array of '#options' form a Drupal select element gets passed though check_plain() inside the form_select_options() function in includes/form.inc. So the option values should not be passed through check_plain() when defining the select form element.

The code at line 1648 to 1650 in commerce_cart.module should be changed from:

        foreach ($products as $product_id => $product) {
          $options[$product_id] = check_plain($product->title);
        }

to

        foreach ($products as $product_id => $product) {
          $options[$product_id] = $product->title;
        }

and the code from 1676 to 1678 should similarly read as follows, without the check_plain():

        foreach ($products as $product_id => $product) {
          $options[$product_id] = $product->title;
        }

Edit: This is the fix for the comment above, which isn't actually the same bug as originally described in this issue. To be more correct I've created a new issue for the Add to cart form bug that this solves here: #1365202: Add to Cart form select list double-escapes HTML entities

Marked #1365202: Add to Cart form select list double-escapes HTML entities as a duplicate.

Regarding #7, I think this introduces an xss risk because essentially you are just reversing the check_plain. To see this, try dpm(htmlspecialchars_decode(check_plain('<script>alert("hi")</script>'))); - it will print the script (although dpm() makes it harmless by wrapping it in <pre> tags).

My instinct would be to use filter_xss($input, array()) to filter the titles that get displayed in this and similar cases, passing in an empty array for the $allow_tags parameter to ensure that no html gets inserted inside option tags.

Ok, I did commit Fonant's fix with documentation in #1365202: Add to Cart form select list double-escapes HTML entities. This issue is concerned with the select list options for the product reference field, and I did reproduce the problem of double sanitization in commerce_product_reference_options_list(). However, it's worth noting that this one hook is used to generate the options list for select lists, which will have their options sanitized, and radio buttons / checkboxes, which won't. This means we need to change this is in such a way that it preserves sanitization unless we detect the $instance['widget']['type'] == 'options_select'. That's what I've done, and it's worth noting that you actually are responsible for sanitizing optgroup values even in select lists.

In testing I also turned up a similar issue in the menu item title for adding products where we were sanitizing the product type title in the menu item even though it gets sanitized by Drupal as a menu item and a page title.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/3171623