Posted by Gurpartap Singh on May 30, 2009 at 10:33pm
| Project: | Bot |
| Version: | 6.x-1.1 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Since the first day Lampoon#jquery couldn't tell what the user said last time. All of a sudden sometimes it would work.
After debugging today, it turned out that bot_seen_all_nicks_for_regexp() was quoting symbols in nicks for regex before separating out the first letter of the nick.
So for some nicks it turned out to be like:
<?php
/**
* Return an array of all current nicks.
*/
function bot_seen_all_nicks_for_regexp() {
...
$nick_name = preg_quote($nick_name, '/');
$nick_name = preg_replace('/^(.)(.*)/', '(\1)(\2)', $nick_name);
$nicks[] = '/\b' . $nick_name . '\b/i';
...
}
?>Converts [domon] int /\b(\)([domon\])\b/i rather than, /\b(\[)(domon\])\b/i. See the (\) becoming the block in the first example, which should rather contain escaped [.
The fix is just to reverse the quote and replace lines, so we first separate out the nicks, then quote to escape the chars:
<?php
/**
* Return an array of all current nicks.
*/
function bot_seen_all_nicks_for_regexp() {
...
$nick_name = preg_replace('/^(.)(.*)/', '(\1)(\2)', $nick_name);
$nick_name = preg_quote($nick_name, '/');
$nicks[] = '/\b' . $nick_name . '\b/i';
...
}
?>
Comments
#1
critical, since the claimed feature won't work if users have such nicks in the room.
These were the users in #jquery few hours back: http://pastie.org/495069.txt (search for
(\)).#2
Totally see the logic with all this, and agree the fix is sound.
Will get it in when I can.
#3
Won't that escape the newly added ()'s too?
how bout
<?php/**
* Return an array of all current nicks.
*/
function bot_seen_all_nicks_for_regexp() {
...
$nick_name = preg_quote($nick_name, '/');
$nick_name = preg_replace('/^([^\\\\]|\\\\.)(.*)/', '(\1)(\2)', $nick_name);
$nicks[] = '/\b' . $nick_name . '\b/i';
...
}
?>
EDIT: ugh, someone linked this in IRC and I didn't notice how old it was... sorry for the noise
... though actually, that code is still there. heh.
#4
#5
An alternate fix to this has been committed. Thanks.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.