Bug 1071166 - Outgoing messages not escaped correctly. r=aleth,clokep,florian
authorArlo Breault <arlolra@gmail.com>
Fri, 28 Nov 2014 09:43:18 -0800
changeset 21587 90bc651566be85aeff9ae8baed127314e1fd3c39
parent 21586 4608f509dc908b60ebda718a84154bb80e7e5c9d
child 21588 d6f2f1a42c74a03dd57f244ccd171d428534893a
push id1305
push usermbanner@mozilla.com
push dateMon, 23 Feb 2015 19:48:12 +0000
treeherdercomm-beta@3ae4f13858fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaleth, clokep, florian
bugs1071166
Bug 1071166 - Outgoing messages not escaped correctly. r=aleth,clokep,florian
chat/components/public/imIConversationsService.idl
chat/components/public/prplIConversation.idl
chat/components/src/imConversations.js
chat/modules/jsProtoHelper.jsm
chat/protocols/irc/irc.js
chat/protocols/xmpp/xmpp.jsm
--- a/chat/components/public/imIConversationsService.idl
+++ b/chat/components/public/imIConversationsService.idl
@@ -64,16 +64,21 @@ interface imIConversationsService: nsISu
 
   nsISimpleEnumerator getConversations();
   prplIConversation getConversationById(in unsigned long aId);
   prplIConversation getConversationByNameAndAccount(in AUTF8String aName,
                                                     in imIAccount aAccount,
                                                     in boolean aIsChat);
 };
 
+// Because of limitations in libpurple (write_conv is called without context),
+// there's an implicit contract that whatever message string the conversation
+// service passes to a protocol, it'll get back as the originalMessage when
+// "new-text" is notified. This is required for the OTR extensions to work.
+
 // A cancellable outgoing message. Before handing a message off to a protocol,
 // the conversation service notifies observers (typically add-ons) of an
 // outgoing message, which can be transformed or cancelled.
 [scriptable, uuid(4391ba5c-9566-41a9-bb9b-fd0a0a490c2c)]
 interface imIOutgoingMessage: nsISupports {
            attribute AUTF8String message;
            attribute boolean cancelled;
   readonly attribute prplIConversation conversation;
--- a/chat/components/public/prplIConversation.idl
+++ b/chat/components/public/prplIConversation.idl
@@ -5,16 +5,17 @@
 
 #include "nsISupports.idl"
 #include "nsISimpleEnumerator.idl"
 #include "nsIObserver.idl"
 
 interface prplIAccountBuddy;
 interface imIAccount;
 interface imIOutgoingMessage;
+interface imIMessage;
 interface nsIURI;
 interface nsIDOMDocument;
 interface prplIChatRoomFieldValues;
 
 /*
  * This is the XPCOM purple conversation component, a proxy for PurpleConversation.
  */
 
@@ -47,16 +48,21 @@ interface prplIConversation: nsISupports
   void sendMsg(in AUTF8String aMsg);
 
   /* Preprocess messages before they are sent (eg. split long messages).
      Can return null if no changes are to be made. */
   void prepareForSending(in imIOutgoingMessage aMsg,
                          [optional] out unsigned long aMsgCount,
                          [retval, array, size_is(aMsgCount)] out wstring aMsgs);
 
+  /* Postprocess messages before they are displayed (eg. escaping). The
+     implementation can set aMsg.displayMessage, otherwise the originalMessage
+     is used. */
+  void prepareForDisplaying(in imIMessage aMsg);
+
   /* Send information about the current typing state to the server.
      aString should contain the content currently in the text field. The
      protocol should return the number of characters that can still be typed. */
   long sendTyping(in AUTF8String aString);
   const long NO_TYPING_LIMIT = 2147483647; // max int = 2 ^ 31 - 1
 
   /* Un-initialize the conversation. Will be called by
      purpleCoreService::RemoveConversation when the conversation is
--- a/chat/components/src/imConversations.js
+++ b/chat/components/src/imConversations.js
@@ -403,16 +403,17 @@ UIConversation.prototype = {
     this._observers = this._observers.filter(function(o) o !== aObserver);
   },
   notifyObservers: function(aSubject, aTopic, aData) {
     if (aTopic == "new-text") {
       aSubject = new imMessage(aSubject);
       this.notifyObservers(aSubject, "received-message");
       if (aSubject.cancelled)
         return;
+      aSubject.conversation.prepareForDisplaying(aSubject);
 
       this._messages.push(aSubject);
       ++this._unreadMessageCount;
       if (aSubject.incoming && !aSubject.system) {
         ++this._unreadIncomingMessageCount;
         if (!this.isChat || aSubject.containsNick)
           ++this._unreadTargetedMessageCount;
       }
@@ -430,16 +431,21 @@ UIConversation.prototype = {
       if (aSubject.incoming && !aSubject.system &&
           (!this.isChat || aSubject.containsNick)) {
         this.notifyObservers(aSubject, "new-directed-incoming-message", aData);
         Services.obs.notifyObservers(aSubject, "new-directed-incoming-message", aData);
       }
     }
   },
 
+  // Used above when notifying of new-texts originating in the
+  // UIConversation. This happens when this.systemMessage() is called. The
+  // conversation for the message is set as the UIConversation.
+  prepareForDisplaying: function(aMsg) {},
+
   // prplIConvIM
   get buddy() this.target.buddy,
   get typingState() this.target.typingState,
   sendTyping: function(aString) this.target.sendTyping(aString),
 
   // Chat only
   getParticipants: function() this.target.getParticipants(),
   get topic() this.target.topic,
--- a/chat/modules/jsProtoHelper.jsm
+++ b/chat/modules/jsProtoHelper.jsm
@@ -477,16 +477,17 @@ const GenericConversationPrototype = {
         observer.observe(aSubject, aTopic, aData);
       } catch(e) {
         this.ERROR(e);
       }
     }
   },
 
   prepareForSending: function(aOutgoingMessage, aCount) null,
+  prepareForDisplaying: function(aImMessage) {},
   sendMsg: function(aMsg) {
     throw Cr.NS_ERROR_NOT_IMPLEMENTED;
   },
   sendTyping: function(aString) Ci.prplIConversation.NO_TYPING_LIMIT,
 
   close: function() {
     Services.obs.notifyObservers(this, "closing-conversation", null);
     Services.conversations.removeConversation(this);
--- a/chat/protocols/irc/irc.js
+++ b/chat/protocols/irc/irc.js
@@ -126,16 +126,20 @@ const GenericIRCConversation = {
   getMaxMessageLength: function() {
     // Build the shortest possible message that could be sent to other users.
     let baseMessage = ":" + this._account._nickname + this._account.prefix +
                       " " + this._account.buildMessage("PRIVMSG", this.name) +
                       " :\r\n";
     return this._account.maxMessageLength -
            this._account.countBytes(baseMessage);
   },
+  // Apply CTCP formatting before displaying.
+  prepareForDisplaying: function(aMsg) {
+    aMsg.displayMessage = ctcpFormatToHTML(aMsg.displayMessage);
+  },
   prepareForSending: function(aOutgoingMessage, aCount) {
     // Split the message by line breaks and send each one individually.
     let messages = aOutgoingMessage.message.split(/[\r\n]+/);
 
     let maxLength = this.getMaxMessageLength();
 
     // Attempt to smartly split a string into multiple lines (based on the
     // maximum number of characters the message can contain).
@@ -274,24 +278,16 @@ ircChannel.prototype = {
   _receivedInitialMode: false,
   // For IRC you're not in a channel until the JOIN command is received, open
   // all channels (initially) as left.
   _left: true,
   // True while we are rejoining a channel previously parted by the user.
   _rejoined: false,
   banMasks: [],
 
-  // Overwrite the writeMessage function to apply CTCP formatting before
-  // display.
-  writeMessage: function(aWho, aText, aProperties) {
-    GenericConvChatPrototype.writeMessage.call(this, aWho,
-                                               ctcpFormatToHTML(aText),
-                                               aProperties);
-  },
-
   // Section 3.2.2 of RFC 2812.
   part: function(aMessage) {
     let params = [this.name];
 
     // If a valid message was given, use it as the part message.
     // Otherwise, fall back to the default part message, if it exists.
     let msg = aMessage || this._account.getString("partmsg");
     if (msg)
@@ -604,24 +600,16 @@ function ircConversation(aAccount, aName
   // Always request the info as it may be out of date.
   this._waitingForNick = true;
   this.requestBuddyInfo(aName);
 }
 ircConversation.prototype = {
   __proto__: GenericConvIMPrototype,
   get buddy() this._account.buddies.get(this.name),
 
-  // Overwrite the writeMessage function to apply CTCP formatting before
-  // display.
-  writeMessage: function(aWho, aText, aProperties) {
-    GenericConvIMPrototype.writeMessage.call(this, aWho,
-                                             ctcpFormatToHTML(aText),
-                                             aProperties);
-  },
-
   unInit: function() {
     this.unInitIRCConversation();
     GenericConvIMPrototype.unInit.call(this);
   },
 
   updateNick: function(aNewNick) {
     this._name = aNewNick;
     this.notifyObservers(null, "update-conv-title");
--- a/chat/protocols/xmpp/xmpp.jsm
+++ b/chat/protocols/xmpp/xmpp.jsm
@@ -32,16 +32,21 @@ XPCOMUtils.defineLazyServiceGetter(this,
 XPCOMUtils.defineLazyServiceGetter(this, "UuidGenerator",
                                    "@mozilla.org/uuid-generator;1",
                                    "nsIUUIDGenerator");
 
 XPCOMUtils.defineLazyGetter(this, "_", function()
   l10nHelper("chrome://chat/locale/xmpp.properties")
 );
 
+XPCOMUtils.defineLazyGetter(this, "TXTToHTML", function() {
+  let cs = Cc["@mozilla.org/txttohtmlconv;1"].getService(Ci.mozITXTToHTMLConv);
+  return function(aTxt) cs.scanTXT(aTxt, cs.kEntities);
+});
+
 /* This is an ordered list, used to determine chat buddy flags:
  *  index < member    -> noFlags
  *  index = member    -> voiced
  *          moderator -> halfOp
  *          admin     -> op
  *          owner     -> founder
  */
 const kRoles = ["outcast", "visitor", "participant", "member", "moderator",
@@ -255,16 +260,23 @@ const XMPPConversationPrototype = {
       who = this._account._connection._jid.jid;
     if (!who)
       who = this._account.name;
     let alias = this.account.alias || this.account.statusInfo.displayName;
     this.writeMessage(who, aMsg, {outgoing: true, _alias: alias});
     delete this._typingState;
   },
 
+  /* Perform entity escaping before displaying the message. We assume incoming
+     messages have already been escaped, and will otherwise be filtered. */
+  prepareForDisplaying: function(aMsg) {
+    if (aMsg.outgoing && !aMsg.system)
+      aMsg.displayMessage = TXTToHTML(aMsg.displayMessage);
+  },
+
   /* Called by the account when a messsage is received from the buddy */
   incomingMessage: function(aMsg, aStanza, aDate) {
     let from = aStanza.attributes["from"];
     this._targetResource = this._account._parseJID(from).resource;
     let flags = {};
     if (aStanza.attributes["type"] == "error") {
       if (!aMsg) {
         // Failed outgoing message unknown.
@@ -907,19 +919,17 @@ const XMPPAccountPrototype = {
       // Prefer HTML (in <html><body>) and use plain text (<body>) as fallback.
       let htmlBody = aStanza.getElement(["html", "body"]);
       if (htmlBody)
         body = htmlBody.innerXML;
       else {
         // Even if the message is in plain text, the prplIMessage
         // should contain a string that's correctly escaped for
         // insertion in an HTML document.
-        body = Cc["@mozilla.org/txttohtmlconv;1"]
-                 .getService(Ci.mozITXTToHTMLConv)
-                 .scanTXT(b.innerText, Ci.mozITXTToHTMLConv.kEntities);
+        body = TXTToHTML(b.innerText);
       }
     }
     if (body) {
       let date;
       let delay = aStanza.getElement(["delay"]);
       if (delay && delay.uri == Stanza.NS.delay) {
         if (delay.attributes["stamp"])
           date = new Date(delay.attributes["stamp"]);