modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

Using Memberof with conditionals generates 500 error #13167

Closed wandering213 closed 7 years ago

wandering213 commented 7 years ago

Summary

When I try to use conditionals to output if a user is a member of a group I get a 500 error. If I don't include 'then/else the error is gone.

Step to reproduce

[[!+modx.user.id:mo=`GroupName`:eq=`1`:then=`True`:else=`False`]]

Observed behavior

Generates a 500 server error

Expected behavior

I would expect it to output True or False

Environment

MODX version 2.5.1, nginx 1.11.5, mysql 5.7.16, Ubuntu 16.04

pixelchutes commented 7 years ago

What happens if you remove the

:eq=`1`

?

wandering213 commented 7 years ago

That resolves the error.

OptimusCrime commented 7 years ago

Without the eq statement, is the behavior still correct?

wandering213 commented 7 years ago

Yes

Jako commented 7 years ago

A conditional could not combined directly with the next conditional as far as I know. There has to be a some conjunction between like and: or or:. But the Internal Server Error is somehow a bug and should be removed.

OptimusCrime commented 7 years ago

My thoughts exactly.

OptimusCrime commented 7 years ago

Traced the bug down to lines like these: https://github.com/modxcms/revolution/blob/2.x/core/model/modx/filters/modoutputfilter.class.php#L145

Conditional arrays should be [1, '||', 1], [0, '&&', 1], and similar. Problem happens when they do not follow the format [value, operand, value, operand, value] and so on. In this case the array is [1, 1] (two true after each other), which fails the eval function.

I am going to submit a PR once I get a chance, but I think the approach would be to loop the array and make sure we have a int first and last, and alternating operands and ints between those.

Mark-H commented 7 years ago

Fixed in 2.6, where by "fixed" I mean it now logs the error "Encountered incorrect use of output conditions" when you have an invalid tag, instead of failing with a fatal error.