1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Fixed Error trying to reset style property

Discussion in 'Resolved Bug Reports' started by Arty, Sep 3, 2014.

  1. Arty

    Arty Well-Known Member

    I tried resetting style property @nodeStats and got this error:

    Server Error
    Array to string conversion
        strval() in XenForo/Model/StyleProperty.php at line 1228
        XenForo_Model_StyleProperty->updateStylePropertyValue() in XenForo/Model/StyleProperty.php at line 1311
        XenForo_Model_StyleProperty->saveStylePropertiesInStyleFromInput() in XenForo/ControllerAdmin/Style.php at line 362
        XenForo_ControllerAdmin_Style->actionStylePropertiesSave() in XenForo/FrontController.php at line 347
        XenForo_FrontController->dispatch() in XenForo/FrontController.php at line 134
        XenForo_FrontController->run() in /Users/kasutaja/Documents/htdocs/xf/www/dev/admin.php at line 13
    Tested in 1.4.0 RC 2

    Edit: actually its not @nodeStats that is causing that issue. Simply saving style properties in nodes list group in admin control panel is triggering that error.
    Last edited: Sep 4, 2014
  2. Chris D

    Chris D XenForo Developer Staff Member

    What is the error? You have included the stack trace without the error message.
  3. Arty

    Arty Well-Known Member

    Sorry, forgot to add. Fixed.
  4. Arty

    Arty Well-Known Member

    I've added check to dump $definition and $newValue before that line when $newValue is not a string, here is property that is causing problems:

    $definition dump:
        [property_definition_id] => 23381
        [definition_style_id] => 0
        [group_name] => responsive
        [title] => Non-Responsive Minimum Width
        [description] => If the responsive design is disabled globally or for a specific page, this will define the minimum width of the page.
        [property_name] => nonResponsiveMinWidth
        [property_type] => scalar
        [css_components] => a:0:{}
        [scalar_type] => number
        [scalar_parameters] => 
        [addon_id] => XenForo
        [display_order] => 100
        [sub_group] => nonresponsive
        [property_id] => 39809
        [style_id] => 0
        [property_value] => 976px
        [effectiveState] => default
    $newValue dump:
        [border-top-width] => 
        [border-top-left-radius] => 
        [border-right-color] => 
        [border-right-style] => 
        [border-right-width] => 
        [border-top-right-radius] => 
        [border-bottom-color] => 
        [border-bottom-style] => 
        [border-bottom-width] => 
        [border-bottom-right-radius] => 
        [border-left-color] => 
        [border-left-style] => 
        [border-left-width] => 
        [border-bottom-left-radius] => 
        [margin-all] => 
        [margin-top] => 1px
        [margin-right] => 4px
        [margin-bottom] => 1px
        [margin-left] => 4px
        [padding-all] => 
        [padding-top] => 
        [padding-right] => 
        [padding-bottom] => 
        [padding-left] => 
        [width] => 14px
        [height] => 14px
        [extra] => display: block;
    white-space: nowrap;
    text-indent: 9999px;
    overflow: hidden;
    opacity: 0.25;
    nonResponsiveMinWidth isn't even customized in that style and its not in nodes list group, so that doesn't make any sense.
  5. Arty

    Arty Well-Known Member

    Found property with that value: @nodeTinyIcon.

    Checked it to make sure there is no mix up in definitions, it has its own correct definition with id 23377. It doesn't make any sense why would XenForo use definition from different style property.
  6. Arty

    Arty Well-Known Member

    Found where error is happening: XenForo_Application::ParseQueryString on this line (line 1245):
            $output = array_merge_recursive($output, $values);
    I've attached dumps (via var_export to help debugging) of variables $chunk, $values and $output inside that loop as well as string value of $string

    There is no mention of number 23381 anywhere in string, anywhere in $chunk and anywhere in $values, yet it magically appears as array key in $output

    Attached Files:

    Mike and Chris D like this.
  7. Mike

    Mike XenForo Developer Staff Member

    Ugh, the whole max_input_vars thing is really really annoying -- why it affects parse_str, I'll never know...
  8. Arty

    Arty Well-Known Member

    I figured that out. Problem is array keys are numbers, not strings. From array_merge_recursive description:
    So because both old $output and $values have numeric key 23377, instead of merging arrays it creates new key that just happens to match definition id of another property.
  9. Arty

    Arty Well-Known Member

    Quick fix: replaced that line with
            $output = self::mergeArrays($output, $values);
    and added new function:
      public static function mergeArrays()
         $arrays = func_get_args();
         $result = array_shift($arrays);
         foreach ($arrays as $array)
           if (!is_array($array))
           foreach ($array as $key => $value)
             if (!isset($result[$key]) || !is_array($result[$key]) || !is_array($value))
               $result[$key] = $value;
               $result[$key] = self::mergeArrays($result[$key], $value);
         return $result;
    Edit: updated with better code that is shorter and checks if arguments are arrays to avoid errors.
    Last edited: Sep 4, 2014
  10. Mike

    Mike XenForo Developer Staff Member

    You actually just wrote XenForo_Application::mapMerge. :)

    There's a situation where that doesn't give the correct output. The problem with chunking this comes down to input in the form of:
    The expected output is:
        [a] => Array
                [0] => 0
                [1] => 1
                [2] => 2
                [3] => 3
    If you force chunks of 1, the array_merge_recursive option gives the correct output. With mapMerge, this output is generated:
        [a] => Array
                [0] => 3
    So mapMerge fails if ...[] spans multiple chunks; array_merge_recursive fails if ...[123]... spans multiple chunks. The former is probably less common, but both could happen.

    When I wrote this code, I did look at writing it by hand, but there are so many edge cases. I don't know if I need to look at it again.
  11. Arty

    Arty Well-Known Member

    Maybe simpler solution would be to change array keys to string for style properties? Like properties[prop12345] instead of properties[12345]
  12. Mike

    Mike XenForo Developer Staff Member

    That would still fall over on other forms -- this is a technique that would be used on any potentially big form.

    I have come up with a workaround that isn't 100% perfect, but it eliminates one of the common failure points. The problem with the mapMerge approach had to do with x[]=0&x[]=1... getting split over chunks. I'm now detecting these variables and automatically replacing the [] with a continuous set of numbers. The only time I believe this should fail is if you have a mix of "push" values and actual values, such as x[0]=1&x[]=2 . I don't think this should be that common though.
    thedude likes this.

Share This Page