Dripal Version: 7.17
Ubercart: 3.2
Plain clean installation

When creating new option:

Warning: Illegal string offset '#value' in uc_attribute_option_form_validate() (line 444 of /var/www/lab.breins.net/htdocs/sites/all/modules/ubercart/uc_attribute/uc_attribute.admin.inc)

Bug found in this function:

function uc_attribute_option_form_validate($form, &$form_state) {
  $pattern = '/^-?\d*(\.\d*)?$/';
  $price_error = t('This must be in a valid number format. No commas and only one decimal point.');
  if (!is_numeric($form_state['values']['weight']['#value']) && !preg_match($pattern, $form_state['values']['weight']['#value'])) {
    form_set_error('weight', $price_error);
  }
}

$form_state['values']['weight']['#value'] doesn't exists

Should be

$form_state['values']['weight']

Couldnt' find a dup bug, but this is my first bug submitted at drupal so my appologies if I looked in the wrong place.

Juan Breinlinger

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

juan.brein’s picture

Issue summary: View changes

Fixed bug report

DanZ’s picture

Status: Active » Needs review
FileSize
804 bytes

Thanks for the report!

When you submit a bug report containing a fix, it helps enormously if you submit a patch (which applies the fix). You attach the patch file to your message then set the status of the issue to "needs review". The testbot will try it out and report certain glaring errors. After someone tests it and reports that it works for them, it can be committed to the repository by a maintainer. Once it's committed, it quickly goes into a -dev release, and eventually goes to the next recommended release.

However, since this is your first bug report, I'll make a patch for you which contains your fix.

See https://drupal.org/project/ubercart/git-instructions for the details about creating a patch with GIT. That's the easiest way, once you have it set up. If that's too much, there are ways to do it without setting up GIT.

TR’s picture

Status: Needs review » Needs work

What's the sequence for reproducing this? I never see this warning when creating attribute options.

That validate function was originally used mainly to validate prices, hence the variable names, but that became unnecessary when we introduced a uc_price form element that contains its own validation. Because we use weights all over the place, I think we should also create a uc_weight form element to make sure weights are validated and validated in the same way every place they can be input, not just here on the option form. So as long as this has been raised as an issue, rather than just a band-aid solution to hide the warning I would like to see a patch that introduces a uc_weight form element and modifies all weight usage for this new element. Then we can eliminate uc_attribute_option_form_validate() entirely.

longwave’s picture

I don't see this either, and although the code is wrong, the validation still works in some circumstances. It looks like $form_state['values']['weight']['#value'] is treated as $form_state['values']['weight'][0] so only the first character is actually validated.

juan.brein’s picture

Thank you Danz for the patch!

Steps to reproduce the bug:

- Plain drupal installation using drush dl drupal version 7.17
- Install ubercart: drush dl ubercart
- Enable attribute: drush en uc_attribute (dependencies are enabled too)
- Add new "test" attribute
- Add new "test" option

Warning: Illegal string offset '#value' in uc_attribute_option_form_validate() (line 444 of /var/www/lab.breins.net/htdocs/sites/all/modules/ubercart/uc_attribute/uc_attribute.admin.inc).

You can test it yourself, I'll send you the credentials on PM

Let me know if you want me to do more testings in my installation.

Juan

longwave’s picture

Ah, I think that warning is new in PHP 5.4. In PHP 5.3 the string offset is cast to zero, as I found in #3.

longwave’s picture

Status: Needs work » Fixed

Fix committed. I will open a followup issue to address TR's comments in #2.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Fixed drupal version