From ffbfe9a40ba8bb340e4d22ec0d08e66fc8cd527c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 22 Oct 2014 07:10:13 -0400 Subject: [PATCH] Bug 955678 - message.servername not set when hostname does not have . or : in it (e.g. localhost) [str is undefined in irc.js:839]. r=aleth --- chat/locales/en-US/irc.properties | 1 + chat/protocols/irc/irc.js | 91 +++++++++++++--------- chat/protocols/irc/ircBase.jsm | 88 +++++++++++---------- chat/protocols/irc/ircCTCP.jsm | 40 +++++----- chat/protocols/irc/ircNonStandard.jsm | 12 +-- chat/protocols/irc/ircServices.jsm | 11 ++- chat/protocols/irc/ircUtils.jsm | 2 +- chat/protocols/irc/test/test_ircMessage.js | 52 ++++++++++--- im/test/xpcshell.ini | 2 +- 9 files changed, 176 insertions(+), 123 deletions(-) diff --git a/chat/locales/en-US/irc.properties b/chat/locales/en-US/irc.properties index c95f5e5c1f..98f0941f50 100644 --- a/chat/locales/en-US/irc.properties +++ b/chat/locales/en-US/irc.properties @@ -13,6 +13,7 @@ irc.usernameHint=nick # disconnected because of an error. connection.error.lost=Lost connection with server connection.error.timeOut=Connection timed out +connection.error.invalidUsername=%S is not an allowed username connection.error.invalidPassword=Invalid server password connection.error.passwordRequired=Password required diff --git a/chat/protocols/irc/irc.js b/chat/protocols/irc/irc.js index 318d29e587..b86cc7df60 100644 --- a/chat/protocols/irc/irc.js +++ b/chat/protocols/irc/irc.js @@ -25,20 +25,26 @@ XPCOMUtils.defineLazyModuleGetter(this, "DownloadUtils", * command A string that is the command or response code. * params An array of strings for the parameters. The last parameter is * stripped of its : prefix. - * If the message is from a user: - * nickname The user's nickname. + * origin The user's nickname or the server who sent the message. Can be + * a host (e.g. irc.mozilla.org) or an IPv4 address (e.g. 1.2.3.4) + * or an IPv6 address (e.g. 3ffe:1900:4545:3:200:f8ff:fe21:67cf). * user The user's username, note that this can be undefined. * host The user's hostname, note that this can be undefined. * source A "nicely" formatted combination of user & host, which is * @ or if host is undefined. - * Otherwise if it's from a server: - * servername This is the address of the server as a host (e.g. - * irc.mozilla.org) or an IPv4 address (e.g. 1.2.3.4) or IPv6 - * address (e.g. 3ffe:1900:4545:3:200:f8ff:fe21:67cf). + * + * There are cases (e.g. localhost) where it cannot be easily determined if a + * message is from a server or from a user, thus the usage of a generic "origin" + * instead of "nickname" or "servername". + * + * Inputs: + * aData The raw string to parse, it should already have the \r\n + * stripped from the end. + * aOrigin The default origin to use for unprefixed messages. */ -function ircMessage(aData) { +function ircMessage(aData, aOrigin) { let message = {rawMessage: aData}; - let temp, prefix; + let temp; // Splits the raw string into four parts (the second is required), the command // is required. A raw string looks like: @@ -57,8 +63,6 @@ function ircMessage(aData) { if (!(temp = aData.match(/^(?::([^ ]+) )?([^ ]+)((?: +[^: ][^ ]*)*)? *(?::([\s\S]*))?$/))) throw "Couldn't parse message: \"" + aData + "\""; - // Assume message is from the server if not specified - prefix = temp[1]; message.command = temp[2]; // Space separated parameters. Since we expect a space as the first thing // here, we want to ignore the first value (which is empty). @@ -67,22 +71,24 @@ function ircMessage(aData) { if (temp[4] != undefined) message.params.push(temp[4]); - // The source string can be split into multiple parts as: - // :(server|nickname[[!user]@host]) - // If the source contains a . or a :, assume it's a server name. See RFC - // 2812 Section 2.3 definition of servername vs. nickname. - if (prefix && - (temp = prefix.match(/^([^ !@\.:]+)(?:!([^ @]+))?(?:@([^ ]+))?$/))) { - message.nickname = temp[1]; - message.user = temp[2] || null; // Optional - message.host = temp[3] || null; // Optional - if (message.user) - message.source = message.user + "@" + message.host; - else - message.source = message.host; // Note: this can be null! - } - else if (prefix) - message.servername = prefix; + // Handle the prefix part of the message per RFC 2812 Section 2.3. + + // If no prefix is given, assume the current server is the origin. + if (!temp[1]) + temp[1] = aOrigin; + + // Split the prefix into separate nickname, username and hostname fields as: + // :(servername|(nickname[[!user]@host])) + [message.origin, message.user, message.host] = temp[1].split(/[!@]/); + + // It is occasionally useful to have a "source" which is a combination of + // user@host. + if (message.user) + message.source = message.user + "@" + message.host; + else if (message.host) + message.source = message.host; + else + message.source = ""; return message; } @@ -683,7 +689,8 @@ ircSocket.prototype = { function(aStr) lowDequote[aStr[1]] || aStr[1]); try { - let message = new ircMessage(dequotedMessage); + let message = new ircMessage(dequotedMessage, + this._account._currentServerName); this.DEBUG(JSON.stringify(message) + conversionWarning); if (!ircHandlers.handleMessage(this._account, message)) { // If the message was not handled, throw a warning containing @@ -783,6 +790,10 @@ function ircAccount(aProtocol, aImAccount) { let splitter = this.name.lastIndexOf("@"); this._accountNickname = this.name.slice(0, splitter); this._server = this.name.slice(splitter + 1); + // To avoid _currentServerName being null, initialize it to the server being + // connected to. This will also get overridden during the 001 response from + // the server. + this._currentServerName = this._server; this._nickname = this._accountNickname; this._requestedNickname = this._nickname; @@ -818,6 +829,17 @@ ircAccount.prototype = { // _requestedNickname when a new nick is automatically generated (e.g. by // adding digits). _sentNickname: null, + get username() { + let username; + // Use a custom username in a hidden preference. + if (this.prefs.prefHasUserValue("username")) + username = this.getString("username"); + // But fallback to brandShortName if no username is provided (or is empty). + if (!username) + username = Services.appinfo.name; + + return username; + }, // The prefix minus the nick (!user@host) as returned by the server, this is // necessary for guessing message lengths. prefix: null, @@ -1275,7 +1297,7 @@ ircAccount.prototype = { // The received timestamp is invalid. if (isNaN(sentTime)) { - this.WARN(aMessage.servername + + this.WARN(aMessage.origin + " returned an invalid timestamp from a PING: " + aPongTime); return false; } @@ -1286,8 +1308,8 @@ ircAccount.prototype = { // If the delay is negative or greater than 1 minute, something is // feeding us a crazy value. Don't display this to the user. if (delay < 0 || 60 * 1000 < delay) { - this.WARN(aMessage.servername + - " returned an invalid delay from a PING: " + delay); + this.WARN(aMessage.origin + " returned an invalid delay from a PING: " + + delay); return false; } @@ -1656,14 +1678,7 @@ ircAccount.prototype = { this.changeNick(this._requestedNickname); // Send the user message (section 3.1.3). - let username; - // Use a custom username in a hidden preference. - if (this.prefs.prefHasUserValue("username")) - username = this.getString("username"); - // But fallback to brandShortName if no username is provided (or is empty). - if (!username) - username = Services.appinfo.name; - this.sendMessage("USER", [username, this._mode.toString(), "*", + this.sendMessage("USER", [this.username, this._mode.toString(), "*", this._realname || this._requestedNickname]); }, diff --git a/chat/protocols/irc/ircBase.jsm b/chat/protocols/irc/ircBase.jsm index b8c56b59bd..66aa920e7e 100644 --- a/chat/protocols/irc/ircBase.jsm +++ b/chat/protocols/irc/ircBase.jsm @@ -45,9 +45,8 @@ function privmsg(aAccount, aMessage, aIsNotification) { if (aIsNotification) params.notification = true; aAccount.getConversation(aAccount.isMUCName(aMessage.params[0]) ? - aMessage.params[0] : aMessage.nickname) - .writeMessage(aMessage.nickname || aMessage.servername, - aMessage.params[1], params); + aMessage.params[0] : aMessage.origin) + .writeMessage(aMessage.origin, aMessage.params[1], params); return true; } @@ -93,8 +92,8 @@ function leftRoom(aAccount, aNicks, aChannels, aSource, aReason, aKicked) { function writeMessage(aAccount, aMessage, aString, aType) { let type = {}; type[aType] = true; - aAccount.getConversation(aMessage.servername) - .writeMessage(aMessage.servername, aString, type); + aAccount.getConversation(aMessage.origin) + .writeMessage(aMessage.origin, aString, type); return true; } @@ -186,8 +185,8 @@ var ircBase = { } // Otherwise, just notify the user. this.getConversation(aMessage.params[1]) - .writeMessage(aMessage.nickname, - _("message.inviteReceived", aMessage.nickname, + .writeMessage(aMessage.origin, + _("message.inviteReceived", aMessage.origin, aMessage.params[1]), {system: true}); return true; }, @@ -196,7 +195,7 @@ var ircBase = { // Add the buddy to each channel for each (let channelName in aMessage.params[0].split(",")) { let conversation = this.getConversation(channelName); - if (this.normalize(aMessage.nickname, this.userPrefixes) == + if (this.normalize(aMessage.origin, this.userPrefixes) == this.normalize(this._nickname)) { // If you join, clear the participants list to avoid errors with // repeated participants. @@ -208,7 +207,7 @@ var ircBase = { // If conversation.chatRoomFields is present, the rejoin was due to // an automatic reconnection, for which we already notify the user. if (!conversation._firstJoin && !conversation.chatRoomFields) { - conversation.writeMessage(aMessage.nickname, _("message.rejoined"), + conversation.writeMessage(aMessage.origin, _("message.rejoined"), {system: true}); } delete conversation._firstJoin; @@ -224,14 +223,14 @@ var ircBase = { else { // Don't worry about adding ourself, RPL_NAMREPLY takes care of that // case. - conversation.getParticipant(aMessage.nickname, true); - let msg = _("message.join", aMessage.nickname, aMessage.source || ""); - conversation.writeMessage(aMessage.nickname, msg, {system: true, - noLinkification: true}); + conversation.getParticipant(aMessage.origin, true); + let msg = _("message.join", aMessage.origin, aMessage.source); + conversation.writeMessage(aMessage.origin, msg, {system: true, + noLinkification: true}); } } // If the joiner is a buddy, mark as online. - let buddy = this.buddies.get(aMessage.nickname); + let buddy = this.buddies.get(aMessage.origin); if (buddy) buddy.setStatus(Ci.imIStatusInfo.STATUS_AVAILABLE, ""); return true; @@ -240,11 +239,11 @@ var ircBase = { // KICK *( "," ) *( "," ) [] let comment = aMessage.params.length == 3 ? aMessage.params[2] : null; // Some servers (moznet) send the kicker as the comment. - if (comment == aMessage.nickname) + if (comment == aMessage.origin) comment = null; return leftRoom(this, aMessage.params[1].split(","), - aMessage.params[0].split(","), aMessage.nickname, - comment, true); + aMessage.params[0].split(","), aMessage.origin, comment, + true); }, "MODE": function(aMessage) { // MODE *( ( "+" / "-") *( "i" / "w" / "o" / "O" / "r" ) ) @@ -254,33 +253,32 @@ var ircBase = { // was updated. this.getConversation(aMessage.params[0]) .setMode(aMessage.params[1], aMessage.params.slice(2), - aMessage.nickname || aMessage.servername); + aMessage.origin); return true; } // Otherwise the user's own mode is being returned to them. return this.setUserMode(aMessage.params[0], aMessage.params[1], - aMessage.nickname || aMessage.servername, - !this._userModeReceived); + aMessage.origin, !this._userModeReceived); }, "NICK": function(aMessage) { // NICK - this.changeBuddyNick(aMessage.nickname, aMessage.params[0]); + this.changeBuddyNick(aMessage.origin, aMessage.params[0]); return true; }, "NOTICE": function(aMessage) { // NOTICE // If the message doesn't have a nickname, it's from the server, don't // show it unless the user wants to see it. - if (!aMessage.hasOwnProperty("nickname")) + if (!aMessage.hasOwnProperty("origin")) return serverMessage(this, aMessage); return privmsg(this, aMessage, true); }, "PART": function(aMessage) { // PART *( "," ) [ ] - return leftRoom(this, [aMessage.nickname], aMessage.params[0].split(","), - aMessage.source || "", + return leftRoom(this, [aMessage.origin], aMessage.params[0].split(","), + aMessage.source, aMessage.params.length == 2 ? aMessage.params[1] : null); }, "PING": function(aMessage) { @@ -300,7 +298,7 @@ var ircBase = { } // Otherwise, the ping was from a user command. else - return this.handlePingReply(aMessage.servername, pongTime); + return this.handlePingReply(aMessage.origin, pongTime); }, "PRIVMSG": function(aMessage) { // PRIVMSG @@ -315,22 +313,22 @@ var ircBase = { if (quitMsg.startsWith("Quit: ")) quitMsg = quitMsg.slice(6); // "Quit: ".length // If a quit message was included, show it. - let msg = _("message.quit", aMessage.nickname, + let msg = _("message.quit", aMessage.origin, quitMsg.length ? _("message.quit2", quitMsg) : ""); // Loop over every conversation with the user and display that they quit. this.conversations.forEach(conversation => { if (conversation.isChat && - conversation._participants.has(aMessage.nickname)) { - conversation.writeMessage(aMessage.servername, msg, {system: true}); - conversation.removeParticipant(aMessage.nickname); + conversation._participants.has(aMessage.origin)) { + conversation.writeMessage(aMessage.origin, msg, {system: true}); + conversation.removeParticipant(aMessage.origin); } }); // Remove from the whois table. - this.removeBuddyInfo(aMessage.nickname); + this.removeBuddyInfo(aMessage.origin); // If the leaver is a buddy, mark as offline. - let buddy = this.buddies.get(aMessage.nickname); + let buddy = this.buddies.get(aMessage.origin); if (buddy) buddy.setStatus(Ci.imIStatusInfo.STATUS_OFFLINE, ""); return true; @@ -342,17 +340,19 @@ var ircBase = { "TOPIC": function(aMessage) { // TOPIC [ ] // Show topic as a message. - let source = aMessage.nickname || aMessage.servername; let conversation = this.getConversation(aMessage.params[0]); let topic = aMessage.params[1]; // Set the topic in the conversation and update the UI. - conversation.setTopic(topic ? ctcpFormatToText(topic) : "", source); + conversation.setTopic(topic ? ctcpFormatToText(topic) : "", + aMessage.origin); return true; }, "001": function(aMessage) { // RPL_WELCOME // Welcome to the Internet Relay Network !@ this._socket.resetPingTimer(); - this._currentServerName = aMessage.servername; + // This seems a little strange, but we don't differentiate between a + // nickname and the servername since it can be ambiguous. + this._currentServerName = aMessage.origin; // Clear user mode. this._modes = new Set(); @@ -490,7 +490,7 @@ var ircBase = { "221": function(aMessage) { // RPL_UMODEIS // return this.setUserMode(aMessage.params[0], aMessage.params[1], - aMessage.servername, true); + aMessage.origin, true); }, /* @@ -819,7 +819,7 @@ var ircBase = { // this.getConversation(aMessage.params[1]) .setMode(aMessage.params[2], aMessage.params.slice(3), - aMessage.servername); + aMessage.origin); return true; }, @@ -856,7 +856,7 @@ var ircBase = { // Note that servers reply with parameters in the reverse order from the // above (which is as specified by RFC 2812). this.getConversation(aMessage.params[2]) - .writeMessage(aMessage.servername, + .writeMessage(aMessage.origin, _("message.invited", aMessage.params[1], aMessage.params[2]), {system: true}); return true; @@ -991,7 +991,7 @@ var ircBase = { } else msg = _("message.noBanMasks", aMessage.params[1]); - conv.writeMessage(aMessage.servername, msg, {system: true}); + conv.writeMessage(aMessage.origin, msg, {system: true}); return true; }, "369": function(aMessage) { // RPL_ENDOFWHOWAS @@ -1252,7 +1252,7 @@ var ircBase = { "443": function(aMessage) { // ERR_USERONCHANNEL // :is already on channel this.getConversation(aMessage.params[2]) - .writeMessage(aMessage.servername, + .writeMessage(aMessage.origin, _("message.alreadyInChannel", aMessage.params[1], aMessage.params[2]), {system: true}); return true; @@ -1284,7 +1284,15 @@ var ircBase = { }, "461": function(aMessage) { // ERR_NEEDMOREPARAMS // :Not enough parameters - // TODO + + if (!this.connected) { + // The account has been set up with an illegal username. + this.ERROR("Erroneous username: " + this.username); + this.gotDisconnected(Ci.prplIAccount.ERROR_INVALID_USERNAME, + _("connection.error.invalidUsername", this.user)); + return true; + } + return false; }, "462": function(aMessage) { // ERR_ALREADYREGISTERED diff --git a/chat/protocols/irc/ircCTCP.jsm b/chat/protocols/irc/ircCTCP.jsm index 27b8304220..5722479940 100644 --- a/chat/protocols/irc/ircCTCP.jsm +++ b/chat/protocols/irc/ircCTCP.jsm @@ -99,8 +99,7 @@ function ctcpHandleMessage(aMessage) { "\nin IRC message: " + message.rawMessage); // For unhandled CTCP message, respond with a NOTICE ERRMSG that echoes // back the original command. - this.sendCTCPMessage(message.nickname || message.servername, true, - "ERRMSG", + this.sendCTCPMessage(message.origin, true, "ERRMSG", [message.ctcp.rawMessage, ":Unhandled CTCP command"]); } } @@ -122,16 +121,15 @@ var ctcpBase = { // ACTION // Display message in conversation this.getConversation(this.isMUCName(aMessage.params[0]) ? - aMessage.params[0] : aMessage.nickname) - .writeMessage(aMessage.nickname || aMessage.servername, - "/me " + aMessage.ctcp.param, + aMessage.params[0] : aMessage.origin) + .writeMessage(aMessage.origin, "/me " + aMessage.ctcp.param, {incoming: true}); return true; }, // Used when an error needs to be replied with. "ERRMSG": function(aMessage) { - this.WARN(aMessage.nickname + " failed to handle CTCP message: " + + this.WARN(aMessage.origin + " failed to handle CTCP message: " + aMessage.ctcp.param); return true; }, @@ -156,14 +154,14 @@ var ctcpBase = { let supportedCtcp = [...info].join(" "); this.LOG("Reporting support for the following CTCP messages: " + supportedCtcp); - this.sendCTCPMessage(aMessage.nickname, true, "CLIENTINFO", + this.sendCTCPMessage(aMessage.origin, true, "CLIENTINFO", supportedCtcp); } else { // Received a CLIENTINFO response, store the information for future // use. let info = aMessage.ctcp.param.split(" "); - this.setWhois(aMessage.nickname, {clientInfo: info}) + this.setWhois(aMessage.origin, {clientInfo: info}) } return true; }, @@ -173,14 +171,14 @@ var ctcpBase = { // PING timestamp if (aMessage.command == "PRIVMSG") { // Received PING request, send PING response. - this.LOG("Received PING request from " + aMessage.nickname + + this.LOG("Received PING request from " + aMessage.origin + ". Sending PING response: \"" + aMessage.ctcp.param + "\"."); - this.sendCTCPMessage(aMessage.nickname, true, "PING", + this.sendCTCPMessage(aMessage.origin, true, "PING", aMessage.ctcp.param); return true; } else - return this.handlePingReply(aMessage.nickname, aMessage.ctcp.param); + return this.handlePingReply(aMessage.origin, aMessage.ctcp.param); }, // These are commented out since CLIENTINFO automatically returns the @@ -198,18 +196,18 @@ var ctcpBase = { // TIME // Received a TIME request, send a human readable response. let now = (new Date()).toString(); - this.LOG("Received TIME request from " + aMessage.nickname + + this.LOG("Received TIME request from " + aMessage.origin + ". Sending TIME response: \"" + now + "\"."); - this.sendCTCPMessage(aMessage.nickname, true, "TIME", ":" + now); + this.sendCTCPMessage(aMessage.origin, true, "TIME", ":" + now); } else { // TIME : // Received a TIME reply, display it. // Remove the : prefix, if it exists and display the result. let time = aMessage.ctcp.param.slice(aMessage.ctcp.param[0] == ":"); - this.getConversation(aMessage.nickname) - .writeMessage(aMessage.nickname, - _("ctcp.time", aMessage.nickname, time), + this.getConversation(aMessage.origin) + .writeMessage(aMessage.origin, + _("ctcp.time", aMessage.origin, time), {system: true}); } return true; @@ -227,17 +225,17 @@ var ctcpBase = { // VERSION // Received VERSION request, send VERSION response. let version = Services.appinfo.name + " " + Services.appinfo.version; - this.LOG("Received VERSION request from " + aMessage.nickname + + this.LOG("Received VERSION request from " + aMessage.origin + ". Sending VERSION response: \"" + version + "\"."); - this.sendCTCPMessage(aMessage.nickname, true, "VERSION", version); + this.sendCTCPMessage(aMessage.origin, true, "VERSION", version); } else if (aMessage.command == "NOTICE" && aMessage.ctcp.param.length) { // VERSION #:#:# // Received VERSION response, display to the user. - let response = _("ctcp.version", aMessage.nickname, + let response = _("ctcp.version", aMessage.origin, aMessage.ctcp.param); - this.getConversation(aMessage.nickname) - .writeMessage(aMessage.nickname, response, {system: true}); + this.getConversation(aMessage.origin) + .writeMessage(aMessage.origin, response, {system: true}); } return true; } diff --git a/chat/protocols/irc/ircNonStandard.jsm b/chat/protocols/irc/ircNonStandard.jsm index ad23bd0f81..92a971a1ac 100644 --- a/chat/protocols/irc/ircNonStandard.jsm +++ b/chat/protocols/irc/ircNonStandard.jsm @@ -36,7 +36,7 @@ var ircNonStandard = { // AUTH); in this case, check if the user's nickname is not auth, or the // the message starts with ***. let target = aMessage.params[0].toLowerCase(); - let nickname = aMessage.nickname ? aMessage.nickname.toLowerCase() : ""; + let nickname = aMessage.origin ? aMessage.origin.toLowerCase() : ""; let isAuth = target == "*" || (target == "auth" && (nickname != "auth" || aMessage.params[1].startsWith("***"))); @@ -44,8 +44,8 @@ var ircNonStandard = { // Some servers , e.g. irc.umich.edu, use NOTICE before connection to give // directions to users. if (!this.connected && !isAuth) { - this.getConversation(aMessage.servername) - .writeMessage(aMessage.servername, aMessage.params[1], + this.getConversation(aMessage.origin) + .writeMessage(aMessage.origin, aMessage.params[1], {incoming: true}); return true; } @@ -159,7 +159,7 @@ var ircNonStandard = { // a NOTICE AUTH will follow causing us to send the password. This numeric // is, unfortunately, also sent if you give a wrong password. The // parameter in that case is "Invalid Password". - return aMessage.servername == "irc.znc.in" && + return aMessage.origin == "irc.znc.in" && aMessage.params[1] == "Password required"; }, @@ -175,8 +175,8 @@ var ircNonStandard = { "998": function(aMessage) { // irc.umich.edu shows an ASCII captcha that must be typed in by the user. - this.getConversation(aMessage.servername) - .writeMessage(aMessage.servername, aMessage.params[1], + this.getConversation(aMessage.origin) + .writeMessage(aMessage.origin, aMessage.params[1], {incoming: true, noFormat: true}); return true; } diff --git a/chat/protocols/irc/ircServices.jsm b/chat/protocols/irc/ircServices.jsm index 0555b496ff..b903980a5b 100644 --- a/chat/protocols/irc/ircServices.jsm +++ b/chat/protocols/irc/ircServices.jsm @@ -41,7 +41,7 @@ function ServiceMessage(aAccount, aMessage) { "nickserv": "NickServ" } - let nickname = aAccount.normalize(aMessage.nickname); + let nickname = aAccount.normalize(aMessage.origin); if (nicknameToServiceName.hasOwnProperty(nickname)) aMessage.serviceName = nicknameToServiceName[nickname]; @@ -64,7 +64,7 @@ var ircServices = { // If we automatically reply to a NOTICE message this does not abide by RFC // 2812. Oh well. "NOTICE": function(aMessage) { - if (!ircHandlers.hasServicesHandlers || !aMessage.hasOwnProperty("nickname")) + if (!ircHandlers.hasServicesHandlers || !aMessage.hasOwnProperty("origin")) return false; let message = ServiceMessage(this, aMessage); @@ -146,7 +146,7 @@ var servicesBase = { // The message starts after the channel name, plus [, ] and a space. let message = aMessage.params[1].slice(channel.length + 3); this.getConversation(channel) - .writeMessage(aMessage.nickname, message, params); + .writeMessage(aMessage.origin, message, params); return true; }, @@ -161,9 +161,8 @@ var servicesBase = { else if (text == "*** \u0002End of Message(s) of the Day\u0002 ***") { if (this._showServerTab && this._infoServMotd) { this._infoServMotd.push(text); - this.getConversation(aMessage.servername || this._currentServerName) - .writeMessage(aMessage.servername || aMessage.nickname, - this._infoServMotd.join("\n"), + this.getConversation(aMessage.origin) + .writeMessage(aMessage.origin, this._infoServMotd.join("\n"), {incoming: true}); delete this._infoServMotd; } diff --git a/chat/protocols/irc/ircUtils.jsm b/chat/protocols/irc/ircUtils.jsm index 3b02b5f996..495dd2fb5e 100644 --- a/chat/protocols/irc/ircUtils.jsm +++ b/chat/protocols/irc/ircUtils.jsm @@ -221,7 +221,7 @@ function mIRCColoring(aStack, aInput) { function conversationErrorMessage(aAccount, aMessage, aError, aJoinFailed = false, aRejoinable = true) { let conv = aAccount.getConversation(aMessage.params[1]); - conv.writeMessage(aMessage.servername, _(aError, aMessage.params[1]), + conv.writeMessage(aMessage.origin, _(aError, aMessage.params[1]), {error: true, system: true}); delete conv._pendingMessage; diff --git a/chat/protocols/irc/test/test_ircMessage.js b/chat/protocols/irc/test/test_ircMessage.js index 9f10710d38..dfc13422e6 100644 --- a/chat/protocols/irc/test/test_ircMessage.js +++ b/chat/protocols/irc/test/test_ircMessage.js @@ -20,8 +20,7 @@ const testData = [ "QUIT :Gone to have lunch", ":syrk!kalt@millennium.stealth.net QUIT :Gone to have lunch", "SQUIT tolsun.oulu.fi :Bad Link ?", - // This fails! But do we really care? It wasn't designed to handle server messages. - //":Trillian SQUIT cm22.eng.umd.edu :Server out of control", + ":Trillian SQUIT cm22.eng.umd.edu :Server out of control", "JOIN #foobar", "JOIN &foo fubar", "JOIN #foo,&bar fubar", @@ -120,13 +119,18 @@ function run_test() { add_test(testRFC2812Messages); add_test(testBrokenUnrealMessages); add_test(testNewLinesInMessages); + add_test(testLocalhost); run_next_test(); } +/* + * Test round tripping parsing and then rebuilding the messages from RFC 2812. + */ function testRFC2812Messages() { for each (let expectedStringMessage in testData) { - let message = irc.ircMessage(expectedStringMessage); + // Pass in an empty default origin in order to check this below. + let message = irc.ircMessage(expectedStringMessage, ""); let stringMessage = irc.ircAccount.prototype.buildMessage(message.command, message.params); @@ -134,7 +138,7 @@ function testRFC2812Messages() { // Let's do a little dance here...we don't rebuild the "source" of the // message (the server does that), so when comparing our output message, we // need to avoid comparing to that part. - if (message.servername || message.source) { + if (message.origin) { expectedStringMessage = expectedStringMessage.slice(expectedStringMessage.indexOf(" ") + 1); } @@ -145,6 +149,9 @@ function testRFC2812Messages() { run_next_test(); } +/* + * Test if two objects have the same fields (recursively). + */ function isEqual(aObject1, aObject2) { let result = true; for (let fieldName in aObject1) { @@ -164,28 +171,31 @@ function isEqual(aObject1, aObject2) { // description of what's wrong. function testBrokenUnrealMessages() { let messages = { + // Two spaces after command. ":gravel.mozilla.org 432 #momo :Erroneous Nickname: Illegal characters": { rawMessage: ":gravel.mozilla.org 432 #momo :Erroneous Nickname: Illegal characters", command: "432", params: ["", "#momo", "Erroneous Nickname: Illegal characters"], - servername: "gravel.mozilla.org" + origin: "gravel.mozilla.org" }, + // An extraneous space at the end. ":gravel.mozilla.org MODE #tckk +n ": { rawMessage: ":gravel.mozilla.org MODE #tckk +n ", command: "MODE", params: ["#tckk", "+n"], - servername: "gravel.mozilla.org" + origin: "gravel.mozilla.org" }, + // Two extraneous spaces at the end. ":services.esper.net MODE #foo-bar +o foobar ": { rawMessage: ":services.esper.net MODE #foo-bar +o foobar ", command: "MODE", params: ["#foo-bar", "+o", "foobar"], - servername: "services.esper.net" + origin: "services.esper.net" } }; for (let messageStr in messages) - do_check_true(isEqual(messages[messageStr], irc.ircMessage(messageStr))); + do_check_true(isEqual(messages[messageStr], irc.ircMessage(messageStr, ""))); run_next_test(); } @@ -198,7 +208,7 @@ function testNewLinesInMessages() { rawMessage: ":test!Instantbir@host PRIVMSG #instantbird :First line\nSecond line", command: "PRIVMSG", params: ["#instantbird", "First line\nSecond line"], - nickname: "test", + origin: "test", user: "Instantbir", host: "host", source: "Instantbir@host" @@ -207,7 +217,7 @@ function testNewLinesInMessages() { rawMessage: ":test!Instantbir@host PRIVMSG #instantbird :First line\r\nSecond line", command: "PRIVMSG", params: ["#instantbird", "First line\r\nSecond line"], - nickname: "test", + origin: "test", user: "Instantbir", host: "host", source: "Instantbir@host" @@ -219,3 +229,25 @@ function testNewLinesInMessages() { run_next_test(); } + +// Sometimes it is a bit hard to tell whether a prefix is a nickname or a +// servername. Generally this happens when connecting to localhost or a local +// hostname and is likely seen with bouncers. +function testLocalhost() { + let messages = { + ":localhost 001 clokep :Welcome to the BitlBee gateway, clokep": { + rawMessage: ":localhost 001 clokep :Welcome to the BitlBee gateway, clokep", + command: "001", + params: ["clokep", "Welcome to the BitlBee gateway, clokep"], + origin: "localhost", + user: undefined, + host: undefined, + source: "" + } + }; + + for (let messageStr in messages) + do_check_true(isEqual(messages[messageStr], irc.ircMessage(messageStr))); + + run_next_test(); +} diff --git a/im/test/xpcshell.ini b/im/test/xpcshell.ini index b36abba3cb..a94a4c5538 100644 --- a/im/test/xpcshell.ini +++ b/im/test/xpcshell.ini @@ -6,4 +6,4 @@ [include:chat/components/src/test/xpcshell.ini] [include:chat/protocols/irc/test/xpcshell.ini] [include:chat/protocols/yahoo/test/xpcshell.ini] -[include:extensions/purple/purplexpcom/src/test/xpcshell.ini] +#[include:extensions/purple/purplexpcom/src/test/xpcshell.ini]