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';
  ...
}
CommentFileSizeAuthor
#4 seen_re.patch1.09 KBjohn morahan

Comments

Gurpartap Singh’s picture

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 (\)).

morbus iff’s picture

Totally see the logic with all this, and agree the fix is sound.

Will get it in when I can.

john morahan’s picture

Won't that escape the newly added ()'s too?

how bout

/**
* 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.

john morahan’s picture

StatusFileSize
new1.09 KB
morbus iff’s picture

Status: Needs review » Fixed

An alternate fix to this has been committed. Thanks.

Status: Fixed » Closed (fixed)

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