add a connection between decryption component and view, #P4TB-43

P4TB-43/README.md

@@ -7,7 +7,7 @@ This is one of the fundamental areas where we want to extend
 Thunderbird. We want to be able to transform a received message in
 order to decrypt it and perform other operations if needed.
 
-## Short answer
+## Main documentation
 
 This https://thunderbird.topicbox.com/groups/addons/Tefecd2c30eac581d
 question to the Thunderbird's addon dev community has been important
@@ -16,10 +16,10 @@ search the question from its title "Transforming an incoming message
 through its MIME representation (multipart/encrypted)".
 
 Developing for the decryption workflow can be a bit tricky because the
-logic is encapsulated in a factory that is called by Thuderbird when
-we click on a `multipart/encrypted` message. For most of the tests i
-used a prototype of the `onPgpMime` function listed in the question
-above.
+logic is encapsulated in a component that is created by a Thuderbird's
+factory when we click on a `multipart/encrypted` message. For most of
+the tests i used a prototype of the `onPgpMime` function listed in the
+question above.
 
 I would call `onPgpMime` with functions that returned specific
 messages, so that i could unregister and try again from the command
@@ -27,6 +27,12 @@ line without reinstalling the plugin. Towards the end when it was more
 stable i just called `pEpController.addFactories/removeFactories`,
 which is slower.
 
+### Links
+
+- `README.md` you are reading this
+- `decryption-control-flow-in-enigmail.md` control flow for PGP/MIME in Enigmail, which led to the solution we are actually adopting
+- `from-enigmail-to-pep.md` why Enigmail's solution is not enough for us and how our current approach differs
+
 ## Other contents
 
 The investigation for P4TB-43 was long and i took some notes while
@@ -41,11 +47,9 @@ related to PGP/Inline is larger, it's the first i found and the first
 i tried to use. The code related to PGP/MIME contains the solution we
 are using now.
 
-### Index
+### Links
 
-- `README.md` you are reading this
 - `thunderbird.txt` i started from analysing Thunderbird's code, then i moved to Enigmail
 - `style-and-markup.md` initial search to figure out whether Enigmail is duplicating Thunderbird's markup for attachments
 - `enigmailMessengerOverlay.md` notes about PGP/Inline code in Enigmail. This involves a set of solutions we eventually discarded
-- `decryption-control-flow-in-enigmail.md` control flow for PGP/MIME in Enigmail, which led to the solution we are actually adopting
 - `open-enigmail-questions.md` some open directions for deepening how PGP/MIME currently works in Enigmail

P4TB-43/from-enigmail-to-pep.md

@@ -0,0 +1,42 @@
+
+# Enigmail's solution was not a solution for pEp
+
+In our case getting the multipart body is not enough, we want to pass
+the whole message to the engine as it's written in the topicbox
+question above. Getting the whole message is not easy in the
+component, as the URI we get there does not work with the most common
+methods lise nsIMessenger.msgHdrFromURI.
+
+Another problem with this approach is that after decrypting the
+message we want to use its contents to modify parts of the view, and
+this is not possible from the component interface.
+
+The component can only return a different MIME message that will be
+parsed by the mime proxy and showed by the interface. While the
+component handler is executed, there is no way to refer to the
+thunderbird window.
+
+## Duplicated decryption call
+
+If the component runs in isolation from the view then we need to
+decrypt the message twice, once for passing the decrypted attachments
+as a result of the component handler and a second time for updating
+the parts of the reading window with the contents coming from the
+decrypted message.
+
+## Current solution
+
+We want to connect the decryption handler component with the view
+scope for the reasons explained above. Such a connection is based
+upon some assumptions, for instance we assume that the view will
+always be called after the component in the case of decryption (for
+encryption, the opposite might be true).
+
+The decryption handler is asynchronous and experiments with
+Thunderbird 68.4 show that we can postpone the resolution of the
+handler indefinitely, so we can wait for a promise to be resolved by
+the view.
+
+This is the reason why the component control flow is depending on the
+view control flow.  Every time the decryption component is created it
+adds an handler that can be called by the reading view.

chrome/content/p4tb.js

@@ -139,15 +139,43 @@ pEpController.messageToCompFields = (pEpMessage, compFields) => {
 
 pEpController.mimeFromMessage = emitter.fromMessage.bind(emitter);
 
-pEpController.addFactories = init;
-pEpController.removeFactories = shutdown;
-
-async function decryptionHandler (partialMime, boundary, fromAddress) {
-    let mime = pEpMimeDecrypt.addWrapper(partialMime, boundary);
-    let decoded = await pEpController.messageFromMIME(mime);
-    decoded.from = new pEp.Identity(fromAddress);
-    let [innerMessage, innerMIMEMessage] = await decryptFromMessage(decoded);
-    return innerMIMEMessage;
+pEpController.factories = (() => {
+    let registered = {};
+    let decrypt = pEpMimeDecrypt.init();
+    return {
+        register: () => {
+            /*
+
+              pEpMimeEncrypt is stateless, and the encryption view
+              communicates with it through message header properties
+
+            */
+            registered.encryption = new Factory(pEpMimeEncrypt.Handler);
+            /*
+
+              the `decrypt` object created by pEpMimeDecrypt holds a
+              promise to the last decryption component that was
+              created, which is waiting for contents from a view
+
+            */
+            registered.decryption = new Factory(decrypt.component);
+        },
+        unregister: () => {
+            Object
+                .values(registered)
+                .map(factory => factory.unregister());
+            registered = {};
+        },
+        resolveDecryptComponent: decrypt.resolveComponent.bind(decrypt)
+    }
+})()
+
+function init () {
+    pEpController.factories.register();
+}
+
+function shutdown () {
+    pEpController.factories.unregister();
 }
 
 async function decryptFromMessage (message) {
@@ -177,21 +205,6 @@ async function decryptFromMessage (message) {
     return [innerMessage, innerMIMEMessage];
 }
 
-let factories = [];
-
-function init () {
-    factories = [
-        new Factory(pEpMimeEncrypt.Handler),
-        new Factory(pEpMimeDecrypt.makeComponent(decryptionHandler))
-    ];
-    return factories;
-}
-
-function shutdown () {
-    factories.map(factory=>factory.unregister());
-    factories = [];
-}
-
 Cm.QueryInterface(Ci.nsIComponentRegistrar);
 
 class Factory {

chrome/content/pEpMimeDecrypt.js

@@ -1,8 +1,16 @@
 
-function makeComponent (funct) {
+function makeComponent (waitView) {
     let component = class {
-        onStartRequest (request, ctxt) {
+        constructor () {
+            // a new component is initialised for every new request,
+            // when users click on a received message in the messenger
+            // view. Every component handles only one request, that
+            // means a sequence of one call to `onStartRequest`, one
+            // or more calls to `onDataAvailable` and a final call to
+            // `onStopRequest`
             this.data = "";
+        }
+        onStartRequest (request/*, ctxt*/) {
             this.decoder = request
                 .QueryInterface(Components.interfaces.nsIPgpMimeProxy);
             this.boundary = getBoundary(request.contentType);
@@ -22,20 +30,44 @@ function makeComponent (funct) {
                 return reader.read(count);
             }
         }
-        onStopRequest (request, status) {
-            let messenger = Components.classes["@mozilla.org/messenger;1"]
-                .createInstance(Components.interfaces.nsIMessenger),
-                uri = request.messageURI.spec, // alternatively decoder.messageURI
-                header = messenger.msgHdrFromURI(uri),
-                fromAddress = "test@test.com";
-            // having an `async onStopRequest` would crash Thunderbird
-            // so we use `synchronise` on `funct` which is async
-            let promise = funct(this.data, this.boundary, fromAddress);
+        onStopRequest (/*request, status*/) {
+            let mime = addWrapper(this.data, this.boundary);
+            /* 
+
+               Decryption view-component tunnel, component
+               end. `waitView` gives us a promise to wait until
+               `resolveComponent` receives a function that returns a
+               MIME message. The message can be passed to the MIME
+               proxy's outputDecryptedData in order to trigger updates
+               in the user interface like showing attachments or
+               updating the body
+
+            */
+            let promise = waitView(mime);
+            /*
+
+              Having an `async onStopRequest` would crash
+              Thunderbird. Maybe if we allow `onStopRequest` to
+              terminate, some objects are deleted and we cannot call
+              `decoder` any longer. So we use `synchronise` to pause
+              the execution of `onStopRequest` until the promise is
+              finalised
+
+            */
             let decoded = synchronise(promise);
-            // TB >= 57, for other versions see the body of
-            // MimeDecryptHandler.returnData
-            // https://gitlab.com/enigmail/enigmail/blob/master/package/mimeDecrypt.jsm#L702-706
-            this.decoder.outputDecryptedData(decoded, decoded.length);
+            if (decoded) {
+                /*
+
+                  This way to interface with the proxy decoder works
+                  for thunderbird >= 57, for other versions see the
+                  body of MimeDecryptHandler.returnData
+                  https://gitlab.com/enigmail/enigmail/blob/master/package/mimeDecrypt.jsm#L702-706
+
+                */
+                this.decoder.outputDecryptedData(decoded, decoded.length);
+            } else {
+                console.log("this decryption component received no data from the view");
+            }
             function synchronise (promise) {
                 let inspector = Cc["@mozilla.org/jsinspector;1"].createInstance(Ci.nsIJSInspector),
                     synchronous,
@@ -43,6 +75,7 @@ function makeComponent (funct) {
                     update = asynchronous => synchronous = asynchronous;
                 promise
                     .then(update)
+                    .catch(console.log.bind(console))
                     .finally(unlock);
                 // wait here for the promise to resolve
                 inspector.enterNestedEventLoop(0);
@@ -69,9 +102,117 @@ function addWrapper (decryptedData, boundary) {
         decryptedData;
 }
 
+function promiseWithHandlers () {
+    let resolve,
+        reject,
+        promise = new Promise((resolve_, reject_) => {
+            resolve = resolve_;
+            reject = reject_;
+        });
+    return [promise, resolve, reject];
+}
+
+function init () {
+    /* 
+
+       We need state to synchronise every component invocation
+       with the eventual view rendering.
+
+       As we assume that our view is rendered once after every
+       component call, we could just store the last handler to be
+       called.
+
+       Having a queue helps us in detecting unexpected timing
+       conditions. With current logic, this queue can either be empty
+       or contain one promise.
+
+    */
+    let waiting = false,
+        resolve,
+        reject,
+        lastArguments,
+        component = makeComponent(waitView);
+    return { component, resolveComponent };
+    function waitView (...someArguments) {
+        let promise;
+        if (waiting) {
+            console.log("warning: a decryption component was waiting for a view and it's being discarded");
+            /*
+
+              Several components have been waiting for
+              rendering, each one expecting some data for
+              passing it to their mime decoder proxy.
+
+              We reject former component promises in order not
+              to leave hanging threads. These rejections are
+              handled in component.onStopRequest synchronise
+              with a simple console.log
+              
+            */
+            reject("another component was initialised after this one");
+            waiting = false;
+        }
+        waiting = true;
+        lastArguments = someArguments;
+        [promise, resolve, reject] = promiseWithHandlers();
+        return promise;
+    }
+    function resolveComponent (handler) {
+        if (!handler) {
+            /*
+
+              This branch is useful for troubleshooting. By calling
+              `pEpController.factories.resolveDecryptComponent();`
+              without arguments in the view, you can expect the user
+              interface to show an encrypted message's parts as body
+              and attachments.
+
+              The other code here in `init` handles the arguments in a
+              generic way so it can adapt to cases where waitView's
+              arguments will change. Here in this fallback handler we
+              use the current actual arguments for simplicity.
+
+            */
+            handler = (encryptedMime) => {
+                return Promise.resolve(encryptedMime);
+            }
+        }
+        if (waiting) {
+            resolve(handler(...lastArguments));
+            waiting = false;
+        } else {
+            /* 
+
+               There are no components waiting for the view, we
+               cannot render this part of the message. Maybe the
+               view that called `resolveComponent` is not the right
+               one.
+
+               When users click on a received message in a window with
+               location.href ==
+               'chrome://messenger/content/messenger.xhtml', we can
+               see that a decryption component is called before the
+               view initialisation. In the other cases we get no
+               guarantees about the timing.
+
+               We pass through this branch also when visiting cached
+               messages through the view. In that case the decryption
+               component gets called only the first time users visit
+               the messages
+
+            */
+            console.log("no decryption component waiting to be resolved");
+        }
+    }
+}
+
 var pEpMimeDecrypt = {
-    makeComponent,
-    addWrapper
+    // `init` is the main interface, currently used in p4tb.js
+    init,
+    // `makeComponent` is less complex than `init`. It can help
+    // testing and troubleshooting and it's used in the runtime
+    // experiments
+    makeComponent
 };
 
 const EXPORTED_SYMBOLS = ["pEpMimeDecrypt"];

chrome/content/pepmsghdrview.js

@@ -129,13 +129,13 @@ var pEpHdrView = {
     init: function () {
         console.debug("pEpHdrView: init()", this, window);
         this.onLoadListener();
-        pEpController.addFactories();
+        pEpController.factories.register();
     },
 
     destroy: function () {
         console.debug("pEpHdrView: destroy()");
         ColumnOverlay.destroy();
-        pEpController.removeFactories();
+        pEpController.factories.unregister();
         delete window.pEpHdrView;
         delete window.MessageView;
         delete window.Helper;
@@ -293,11 +293,35 @@ var pEpHdrView = {
             // Store decrypted copy if possible
             storeDecryptedCopy(msgHdr, decryptedMIMEMessage);
 
-
         }).catch(err => {
             console.error("pepmsghdrview.js: onLoadMsgPanelFrameListener(): getMimeDecodedMessage", err)
         });
 
+        /*
+
+          Decryption view-component tunnel, view end. Decryption
+          components wait for a view. Here from the view we pass a
+          function that returns a MIME message, so that the last
+          component can pass the message to Thunderbird's MIME proxy
+          service.
+
+        */
+        pEpController.factories.resolveDecryptComponent();
+
+        async function decryptionHandler (mime) {
+            // When ticket #204 will be closed there will be no need
+            // to call the engine twice here. We can just return one
+            // of the decrypted messages in the scope and ignore the
+            // arguments. Also, this `decryptionHandler` function and
+            // the call to `resolveDecryptComponent` can be moved
+            // within the `messageFromMIME(...).then(...)` branch
+            // above
+            let decoded = await pEpController.messageFromMIME(mime);
+            decoded.longmsg = atob(decoded.attachments[1].value);
+            let result = await pEpController.decryptMailWithMessage(decoded);
+            return "";
+            return result.longmsg;
+        }
     }
 };
 

tests/runtime/pepmsghdrview.js

@@ -28,16 +28,25 @@ let tests = [
         }
     },
     () => {
+        var [promise, resolve, reject] = pEpMimeDecrypt.promiseWithHandlers();
+        function handler (mime) {
+            return promise.then(()=>mime);
+        }
         var component = pEpMimeDecrypt.makeComponent(handler);
-        new pEpController.Factory(component); // this also registers the factory
-        console.log("a new decryption component has been registered, click on a multipart/encrypted message to trigger the handler invocation");
+        var factory = new pEpController.Factory(component);
+        /* 
+
+           A new decryption component has been registered, click on a
+           multipart/encrypted message to trigger the component
+           creation. The component will pause waiting for the
+           handler. Call `resolve()` to finalise the component's
+           lifecycle and see an encrypted message in the user
+           interface. In the extension code we will decrypt the
+           message in the handler
+
+        */
+        factory.unregister();
         return true;
-        function handler (data, boundary, fromAddress) {
-            console.log("decryption component handler called with arguments:");
-            console.log(arguments);
-            let mime = pEpMimeDecrypt.addWrapper(data, boundary);
-            return Promise.resolve(mime);
-        }
     }
 ];