commerce_line_item_form_alter may lead to this core issue: "Warning : array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController" (#1803048: Warning : array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController) as it doesn't check the value given to commerce_order_load() correctly.

In some cases (in mine for anonymous users loading the page) it provides an array with 1 booelan element instead of the order ID to load:
[0] => false

So I'd suggest further checking the value before providing the parameter. See Mr.

commerce_line_item.module, Line 106 ff.

/**
 * Implements hook_form_alter().
 *
 * Alter the views form with functionality specific to line items.
 * This form currently only supports line items from a single order, and it
 * determines which order the line items are for based on a Views argument.
 */
function commerce_line_item_form_alter(&$form, &$form_state, $form_id) {
  $line_item_form = FALSE;

  // Is this a views form?
  if (strpos($form_id, 'views_form_') === 0) {
    $view = $form_state['build_info']['args'][0];

    // Does the view contain one of the line item edit fields?
    foreach ($view->field as $field_name => $field) {
      if ($field instanceof commerce_line_item_handler_field_edit_delete || $field instanceof commerce_line_item_handler_field_edit_quantity) {
        $line_item_form = TRUE;
      }
    }
  }

  // This is not the form we are looking for.
  if (!$line_item_form) {
    return;
  }

  // Require the existence of an order_id argument (and its value).
  if (empty($view->argument['order_id']) || empty($view->argument['order_id']->value)) {
    return;
  }

  $form['#attached']['css'][] = drupal_get_path('module', 'commerce_line_item') . '/theme/commerce_line_item.theme.css';
  $form['#attached']['js'][] = drupal_get_path('module', 'commerce_line_item') . '/commerce_line_item.js';
  $form['#submit'][] = 'commerce_line_item_line_item_views_form_submit';

  // Wrap the form in a div so we can add CSS and javascript behaviors to it.
  $form['#prefix'] = '<div class="commerce-line-item-views-form">';
  $form['#suffix'] = '</div>';

  // Add an additional class to the actions wrapper.
  $form['actions']['#attributes']['class'][] = 'commerce-line-item-actions';

  // Load the order from the Views argument.
  $order = commerce_order_load($view->argument['order_id']->value[0]);
  $form_state['order'] = $order;
}
[1] => Array
(
[file] => /sites/all/modules/contrib/commerce/modules/order/commerce_order.module
[line] => 782
[function] => entity_load
[args] => Array
(
[0] => commerce_order
[1] => Array
(
[0] => false
)

[2] => Array
(
)

[3] =>
)

)

[2] => Array
(
[file] => /sites/all/modules/contrib/commerce/modules/order/commerce_order.module
[line] => 751
[function] => commerce_order_load_multiple
[args] => Array
(
[0] => Array
(
[0] => false
)

[1] => Array
(
)

)

)

[3] => Array
(
[file] => /sites/all/modules/contrib/commerce/modules/line_item/commerce_line_item.module
[line] => 143
[function] => commerce_order_load
[args] => Array
(
[0] => false
)

)

[4] => Array
(
[file] => /includes/module.inc
[line] => 1171
[function] => commerce_line_item_form_alter
[args] => Array

Issue fork commerce-3236711

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Anybody created an issue. See original summary.

anybody’s picture

Status: Active » Needs review

I'm still not sure what this is caused by in the end, but I'm sure we should harden the code to never load an order from a broken order id parameter. :)

Please review the MR.

rszrama’s picture

Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community

No clue how to reproduce, but the cart form still works fine after applying this. 😅

rszrama’s picture

Status: Reviewed & tested by the community » Fixed

  • rszrama committed 635126e on 7.x-1.x authored by Anybody
    Issue #3236711 by Anybody, rszrama: commerce_line_item_form_alter...
rszrama’s picture

Status: Fixed » Closed (won't fix)

Actually, apparently introducing this change causes test failures due to the same errors ... they just appear further down the stream for some reason. I think I'm just gonna revert and leave this be for now. Let's reopen this if we can find a way to reproduce it reliably.

anybody’s picture

Status: Closed (won't fix) » Active

Hi @rszrama,

thank you very very much! :)
I'll try to find out, what caused this. I guess it was a rule or view...

But from having a look at the code I still think it makes a lot of sense and we should check how it can lead to errors down the stream.
Compare

  // Require the existence of an order_id argument (and its value).
  if (empty($view->argument['order_id']) || empty($view->argument['order_id']->value)) {
    return;
  }
 $order = commerce_order_load($view->argument['order_id']->value[0]);

So from my perspective the patch can only harden the logics?

   // Require the existence of an order_id argument (and its value).
-  if (empty($view->argument['order_id']) || empty($view->argument['order_id']->value)) {
+  if (empty($view->argument['order_id']) || empty($view->argument['order_id']->value[0]) || !is_integer($view->argument['order_id']->value[0])) {
     return;
   }

Simply doesn't make sense to call commerce_order_load() with a non-numeric value? And in a form alter hook we shouldn't expect the value to be not empty, I guess...

Can I see the test results online?

rszrama’s picture

Yeah, that's what was so weird ... it looked like as safe a change as possible. Tests were specifically failing against PHP 5:

https://www.drupal.org/pift-ci-job/2221760

anybody’s picture

That is so strange ;D
Especially that the error pops up that was the original issue for me which this patch should fix...

load
exception: [Warning] Line 175 of includes/entity.inc:
array_flip() [function.array-flip]: Can only flip STRING and INTEGER values!

exception: [Warning] Line 175 of includes/entity.inc:
array_flip() [function.array-flip]: Can only flip STRING and INTEGER values!

exception: [Warning] Line 175 of includes/entity.inc:
array_flip() [function.array-flip]: Can only flip STRING and INTEGER values!

The reason for other failing tests is that

$form_state['order']

is empty, which may happen if the condition kicks out a valid value. Can a commerce order (especially in tests) ever have a zero ID? So that the empty() check breaks it?

But honestly, I don't really get it yet...

anybody’s picture

Another follow-up, propably based on the same root cause:
#3332380: $form_state['order'] is empty on cart submit

jsacksick’s picture

Status: Active » Closed (won't fix)

D7 is EOL, so closing this outdated issue.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.