Posted by AlexisWilke on January 23, 2011 at 1:49am
3 followers
| Project: | Rules |
| Version: | 6.x-1.x-dev |
| Component: | Rules Core |
| Category: | bug report |
| Priority: | major |
| Assigned: | AlexisWilke |
| Status: | closed (fixed) |
Issue Summary
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
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| rules-6.x-condition_text_compare_regex.patch | 438 bytes | Idle | PASSED: [[SimpleTest]]: [MySQL] 13 pass(es). | View details |
Comments
#1
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
#2
>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.
#3
Automatically closed -- issue fixed for 2 weeks with no activity.