Sterc / FormIt

A dynamic form processing Snippet for MODX Revolution
https://docs.modx.com/current/en/extras/formit
33 stars 58 forks source link

Email output fails on PHP 7.1 #148

Closed miramar-gareth-parker closed 6 years ago

miramar-gareth-parker commented 6 years ago

Updated PHP on a production site to PHP running latest version of FormIt. The output email does not seem to process the fields value, as the email simply contains a text value of '[[+fields]]'.

Rolled back to PHP 5.6.3 at which point the output emails work again. This site is running in a cPanel environment with EasyApache4, so I have confirmed that the modules loaded into both versions of PHP are the same.

MODX Version: 2.3.3 FormIt Version: 3.0.4-pl PHP Version: 7.1

gpsietzema commented 6 years ago

Hello @miramar-gareth-parker - is that MODX version correct? That is very old. I know updating something like that will take a lot of work, but I would highly recommend it. The least you can do is patch it: https://www.sterc.com/modx/modx-2.5.2-security-patch

We are still on 7.0 environments, but can test a 7.1 in the meantime.

gravinos commented 6 years ago

Hello. I have a same problem here. After switching to php 7.1. formit don't sent a email. Back to 5.6.30 and back to send email.

MODX Version: 2.6.1-pl (latest) FormIt Version: 3.0.4-pl (latest) PHP Version: 7.1

miramar-gareth-parker commented 6 years ago

@gpsietzema we've got an open request to get the site upgraded, however it's a big beast so will take a while! We've already got the 2.5.2 patch in place, so we're hopefully good until we can get the full site updated.

Anyway, found the issue. In /core/components/formit/model/formit/fihooks.class.php on line 387 $f is defined as $f = ''; and items are assigned to the array in the next block. PHP 7.1 has removed pushing array items to a string. Simply changing that row to $f = array(); appears to fix it on PHP 7.1 and above.

I'm happy to put a pull request in, or as it's a simple one line change feel free to implement yourselves.

dannevang commented 6 years ago

@miramar-gareth-parker I think a PR would be good - this definitely need fixing.