• 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

Arty

Well-known member
#1
I tried resetting style property @nodeStats and got this error:

Server Error
Array to string conversion
Code:
    XenForo_Application::handlePhpError()
    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:

Arty

Well-known member
#4
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:
Code:
Array
(
    [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:
Code:
Array
(
    [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.
 

Arty

Well-known member
#5
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.
 

Arty

Well-known member
#6
Found where error is happening: XenForo_Application::ParseQueryString on this line (line 1245):
Code:
        $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
 

Attachments

Mike

XenForo developer
Staff member
#7
Ugh, the whole max_input_vars thing is really really annoying -- why it affects parse_str, I'll never know...
 

Arty

Well-known member
#8
I figured that out. Problem is array keys are numbers, not strings. From array_merge_recursive description:
If, however, the arrays have the same numeric key, the later value will not overwrite the original value, but will be appended.
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.
 

Arty

Well-known member
#9
Quick fix: replaced that line with
Code:
        $output = self::mergeArrays($output, $values);
and added new function:
Code:
  public static function mergeArrays()
   {
     $arrays = func_get_args();
     $result = array_shift($arrays);
     foreach ($arrays as $array)
     {
       if (!is_array($array))
       {
         continue;
       }
       foreach ($array as $key => $value)
       {
         if (!isset($result[$key]) || !is_array($result[$key]) || !is_array($value))
         {
           $result[$key] = $value;
         }
         else
         {
           $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:

Mike

XenForo developer
Staff member
#10
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:
Code:
a[]=0&a[]=1&a[]=2&a[]=3
The expected output is:
Code:
Array
(
    [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:
Code:
Array
(
    [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.
 

Arty

Well-known member
#11
Maybe simpler solution would be to change array keys to string for style properties? Like properties[prop12345] instead of properties[12345]
 

Mike

XenForo developer
Staff member
#12
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.