October: Twig 2.7.4 breaks OctoberCMS w/ RainLab.Pages

[adrenth]

• OctoberCMS Build: 447
• PHP Version: 7.2.16
• Plugin: RainLab.Pages

Description:

The Twig loader causes OctoberCMS to generate a fatal error:
modules/cms/twig/Loader.php:64

Type error: Argument 1 passed to Twig\Source::__construct() must be of the type string, null given, called in ../modules/cms/Twig/Loader.php on line 63

The use of Twig_Source is deprecated as of Twig v2.7, it should be replaced with:

old:

return new Twig_Source($dataHolder->content, $name);

new (suggested fix):

return new Twig\Source((string) $dataHolder->content, $name);

Mention the cast to a string. Obviously the $content some lines above can be NULL and will trigger the error. This is just an example, I think we have to make sure the $dataHolder->content must be a string at all time.

Steps To Reproduce:

• Perform a composer update which will install twig/twig:2.7.4.
• Create a page using Static Pages plugin
• Visit the page

[Balex70]

Facing the same problem after composer update (Twig 7.2.2 -> 7.2.4).
OctoberCMS Build: 447
PHP Version: 7.1.17

[w20k]

@adrenth shouldn't it be as a part of Rainlab.Pages issues or may be linked?

[adrenth]

This is linked to the Pages plugin I found out. But... the return value of getTwigContent() in this particular function should always be a string.

Since OC is not that strict with PHP data types this issue should be resolved by making sure the correct types are provided. Do not make assumptions, but make sure the correct datatype is used.

In this case a cast-to-string solution may fix a lot of future problems.

[seanthepottingshed]

@adrenth Thanks so much!!!!

[tobias-kuendig]

Using a cast on (string) $dataHolder->content is fixing the issue for our projects as well.

[tobias-kuendig]

https://github.com/octobercms/october/pull/4215

Please test and report if it needs any additions.

[w20k]

@adrenth , @seanthepottingshed, @Balex70 - could you guys re-test PR from @tobias-kuendig ?

[Balex70]

For me it works on fresh new installation!

[seanthepottingshed]

@w20k @tobias-kuendig Tested works for me on the project I'm currently working on, thanks so much!

[nizzac]

@tobias-kuendig Fix working for me on an October install updated from 443 to 447.

[tobias-kuendig]

Don't want to be pushy but can we get this merged and released then? We're having more and more customers who trigger updates and break their websites. :sweat_smile:

Also, if I'm not mistaking, this bug also affects all new installations.

[adrenth]

@tobias-kuendig @w20k @LukeTowers Reviewed the PR and it's good to go.

[LukeTowers]

Please test #4218

[w20k]

Bump, guys @adrenth, @seanthepottingshed, @Balex70, if you have a minute, please re-test @LukeTowers PR and will be merged!

Neat trick to get PR patch: https://patch-diff.githubusercontent.com/raw/octobercms/october/pull/4218.patch

[adrenth]

Great PR!

Applied the patch on a fresh project:

• Fetched the latest packages using composer update
• Installed latest Rainlab.Pages plugin.
• Created a page '/test' with some content
• Navigated to page; no error.