437184 - Inline CTCP causes messages to get lost

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[SmoothPorcupine Pirate]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9
Build Identifier: 0.9.82.1

When a message is sent to you or a channel you are in through either PRIVMSG or NOTICE, it is checked for CTCP with this regular expression:
/\x01.*\x01/i
If this regular expression then fails to match, it silently fails:
/^\x01([^ ]+) ?(.*)\x01$/i
No errors, NOTICE's, PRIVMSG's, or any other type of message is displayed.

Reproducible: Always

Steps to Reproduce:
1. Receive a specially crafted message.

Actual Results:  
Nothing at all.

Expected Results:  
Completely awesome inline messages, see attachment.
(Or at least show something.)

Comment 1

[SmoothPorcupine Pirate]

Attachment: Awesome!

Comment 2

[Glen Mailer]

I see what you mean now, couldn't understand a word you were trying to say in the channel the other night.

I believe the fix here is to change /\x01.*\x01/ to /^\x01.*\x01$/ (I cant see why case-insensitive is necessary)

irc.js; CIRCServer.prototype.onPrivmsg
http://mxr.mozilla.org/mozilla/source/extensions/irc/js/lib/irc.js#2434

Comment 3

[Glen Mailer]

Attachment: flag only real CTCP messages as such

Comment 4

[:Gijs (he/him)]

Comment on attachment 323737 [details] [diff] [review]
flag only real CTCP messages as such

Actually, looking at it myself, I think you should use:

/^\x01[^ ]+.*\x01$/

Unless we want to assume that IRC servers don't send on stuff that starts \x01<space>

I don't really like making assumptions about IRC servers too much. :-(

Comment 5

[Glen Mailer]

Attachment: regex a bit stricter

updated as per comments

Comment 6

[:Gijs (he/him)]

Comment on attachment 323750 [details] [diff] [review]
regex a bit stricter

r=me

Comment 7

[Glen Mailer]

Attachment: there was more than one regex!

As pointed out by the OP, the same check is also being done in onNotice, so this patch corrects that one too.

Could probably be refactored a bit to some better DRY principles, but I dont really like to mess with the JS-IRC API, so plastering over this one.

Comment 8

[Glen Mailer]

Attachment: the real one

i'm an idiot, i might even submit a bug to bugzilla to make it ask me twice whenever i try and post an attachment.

Correct patch this time.

Comment 9

[:Gijs (he/him)]

Comment on attachment 323759 [details] [diff] [review]
the real one

r=me

Comment 10

[:Gijs (he/him)]

Checking in mozilla/extensions/irc/js/lib/irc.js;
/cvsroot/mozilla/extensions/irc/js/lib/irc.js,v  <--  irc.js
new revision: 1.124; previous revision: 1.123
done