Closed (fixed)
Project:
Bot
Version:
6.x-1.1
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
30 May 2009 at 22:33 UTC
Updated:
26 May 2010 at 22:10 UTC
Jump to comment: Most recent file
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:
/**
* 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:
/**
* 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';
...
}
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | seen_re.patch | 1.09 KB | john morahan |
Comments
Comment #1
Gurpartap Singh commentedcritical, 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
(\)).Comment #2
morbus iffTotally see the logic with all this, and agree the fix is sound.
Will get it in when I can.
Comment #3
john morahan commentedWon't that escape the newly added ()'s too?
how bout
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.
Comment #4
john morahan commentedComment #5
morbus iffAn alternate fix to this has been committed. Thanks.