Hi Fago,

I was just updating to 1.4 from an older version and noticed that one change in the rules_condition_text_compare() function:

@@ -169,7 +171,10 @@
  * Condition Implementation: Textual comparison.
  */
function rules_condition_text_compare($text1, $text2, $settings) {
-  return $settings['regex'] ? ereg($text2, $text1) : $text1 == $text2;
+  if ($settings['regex']) {
+    return (bool) preg_match('/'. $text2 .'/', $text1);
+  }
+  return $text1 == $text2;
}
/**

I'm not too sure where $text2 comes from, but at this time it is taken as a perl regex entry...

The problem I can see is that you use it bare. So it is viewed as a regex alright, but that expression could include a slash... and in that case it breaks. I advice using the preg_quote() to fix the problem (see patch for fix.)

Now in the following interface, you say "Text 2". May I suggest we reword that "RegEx 2" ? Or at least give a hint to the user that it is a regex...

<?php
   
'rules_condition_text_compare' => array(
     
'label' => t('Textual comparison'),
     
'arguments' => array(
       
'text1' => array('label' => t('Text 1'), 'type' => 'string'),
       
'text2' => array('label' => t('Text 2'), 'type' => 'string'),
      ),
     
'help' => t('TRUE is returned, if both texts are equal.'),
     
'module' => 'Rules',
    ),
?>

Thank you.
Alexis Wilke

Files: 
CommentFileSizeAuthor
#1 rules-6.x-condition_text_compare_regex2.patch446 bytesAlexisWilke
PASSED: [[SimpleTest]]: [MySQL] 13 pass(es).
[ View ]
rules-6.x-condition_text_compare_regex.patch438 bytesAlexisWilke
PASSED: [[SimpleTest]]: [MySQL] 13 pass(es).
[ View ]

Comments

Component:Rules Core» Rules Engine
StatusFileSize
new446 bytes
PASSED: [[SimpleTest]]: [MySQL] 13 pass(es).
[ View ]

Okay... well... I guess if the flag $settings['regex'] means that $text2 is a regex, then we don't want to quote the entire text... However, I still think that the user shouldn't be in the known for the / character.

And according to the description, I wouldn't know...

'#description' => t('If enabled, the matching pattern will be interpreted as a <a href="@regex-wikipedia">regex</a>.  Tip: <a href="@RegExr">RegExr: Online Regular Expression Testing Tool</a> is helpful for learning, writing, and testing Regular Expressions.', array('@regex-wikipedia' => 'http://en.wikipedia.org/wiki/Regular_expression', '@RegExr' => 'http://gskinner.com/RegExr/')),

So there is a new patch that replaces every slash character (/) into a backslashed slash character (\/).

Thank you.
Alexis

Status:Needs review» Fixed

>Okay... well... I guess if the flag $settings['regex'] means that $text2 is a regex, then we don't want to quote the entire text... However, I still think that the user shouldn't be in the known for the / character.
Exactly!

thanks, committed.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.