contao-community-alliance / contao-multicolumnwizard-bundle

Contao 4 Widget - MultiColumnWizard
GNU Lesser General Public License v3.0
12 stars 12 forks source link

Fix illegal array access #106

Closed fritzmg closed 3 years ago

fritzmg commented 3 years ago

Fixes an illegal array access that can occur in MultiColumnWizard::generate:

ErrorException:
Warning: Illegal string offset 'form'

  at vendor\menatwork\contao-multicolumnwizard-bundle\src\Contao\Widgets\MultiColumnWizard.php:792
  at MenAtWork\MultiColumnWizardBundle\Contao\Widgets\MultiColumnWizard->generate()
     (vendor\contao\core-bundle\src\Resources\contao\library\Contao\Widget.php:651)
zonky2 commented 3 years ago

@fritzmg pls check Travis

fritzmg commented 3 years ago

Unfortunately I don't really see what the actual error is. May be someone can point it out to me what needs to be changed?

zonky2 commented 3 years ago
-                $objWidget = $this->initializeWidget($arrField, $i, $strKey, $this->varValue[$i][$strKey] ?? null);
+                $objWidget = $this->initializeWidget($arrField, $i, $strKey, ($this->varValue[$i][$strKey] ?? null));

say Xtra :D

discordier commented 3 years ago

Sadly you got bitten by https://github.com/squizlabs/PHP_CodeSniffer/issues/2325, there is unfortunately nothing we can do here, aside from adding the brackets (even if they seem to be a little over expressive).

baumannsven commented 3 years ago

@discordier Why should we do anything about it? I think this is good

fritzmg commented 3 years ago

Why would you want to have these brackets?

In any case, I think the root cause of this error is may be somewhere else entirely. Under some circumstances MCW saves invalid data into its widget's database field and then that's when this illegal array access occurs. Unfortunately I haven't found a clear reproduction yet.

zonky2 commented 3 years ago

what kind of "invalid data"?

fritzmg commented 3 years ago

In our case the database just contained the string 0, instead of a serialized array.

baumannsven commented 3 years ago

If you have configured the default value and not all column fields are available. This is then such a case study

fritzmg commented 3 years ago

Which case do you mean? The illegal array access for this PR? But why would only specific column fields not be available? Either all or none should be there usually.

baumannsven commented 3 years ago
$GLOBALS['TL_DCA']['table']['fields']['multi'] = [
    'inputType' => 'multiColumnWizard',
    'default'   => [
        0 => [
            'foo' => 'fooValue'
        ]
    ],
    'eval'      => [
        'columnFields' => [
            'foo' => [
                'inputType' => 'text'
            ],
            'bar' => [
                'inputType' => 'text'
            ]
        ]
    ]
];

In this example, the default differs from the fields.

fritzmg commented 3 years ago

Ah ok, yeah but this is not the case for the fields where this problem occurred. No default has been defined there.

stefanheimes commented 3 years ago

PR is merges in the new hotfix/3.4.11. Please test it and let me know if this works @fritzmg. Corrections for travis are done.

fritzmg commented 3 years ago

I made a quick test and it should be fine :)