Loading Penpal with System.js fails to connect

[gajewsk2]

System.js loads assets asynchronously (in development) and this causes the iframe's 'load' event to fire pre-emptively, for the purposes of Penpal. The child's connectToParent function has not been called, so it cannot receieve the message from the parent that is sent after the iframe's load event fires.

We have yet to find a way to cause System.js to load synchronously, except with bundling, which is only used in production. Currently we have developed a work-around that involves exposing a version of the handleIframeLoaded function that only calls the child.postMessage HANDSHAKE part of the function (which we have dubbed 'manualHandshake'). We then have the child send a message to the parent once System.js has done its work, to trigger a re-handshake.

The other solutions we tried, such as trying to re-fiore the Load event, didn't work because we are inside a situation where CORS is blocking us, which is what we wanted Penpal for in the first place.

Do you have any opinions or advice on how to address this issue without a code change to the repository? Alternately, we could submit a pull request with our solution if the solution sounds reasonable and useful. (Current 'issue' with the pull request is that it would change the return interface of connectToChild to return this extra handshaking function as well.)

[gajewsk2]

Penpal.connectToChild = function (_ref) {
  var url = _ref.url,
      appendTo = _ref.appendTo,
      _ref$methods = _ref.methods,
      methods = _ref$methods === undefined ? {} : _ref$methods;

  var destroy = void 0;
  var destructionPromise = new DestructionPromise(function (resolve) {
    return destroy = resolve;
  });

  var parent = window;
  var iframe = document.createElement('iframe');

  (appendTo || document.body).appendChild(iframe);

  destructionPromise.then(function () {
    if (iframe.parentNode) {
      iframe.parentNode.removeChild(iframe);
    }
  });

  var child = iframe.contentWindow || iframe.contentDocument.parentWindow;
  var childOrigin = getOriginFromUrl(url);

// NEW PART
  var manualHandshake = () => {
      child.postMessage({
        penpal: HANDSHAKE,
        methodNames: Object.keys(methods)
      }, childOrigin);
  };
  var promise = new Penpal.Promise(function (resolve, reject) {
    var handleMessage = function handleMessage(event) {
      if (event.source === child && event.origin === childOrigin && event.data.penpal === HANDSHAKE_REPLY) {
        log('Parent: Received handshake reply from Child');

        parent.removeEventListener(MESSAGE, handleMessage);

        var info = {
          localName: PARENT,
          local: parent,
          remote: child,
          remoteOrigin: event.origin
        };

        connectCallReceiver(info, methods, destructionPromise);
        resolve(createCallSender(info, event.data.methodNames, destructionPromise));
      }
    };

    var handleIframeLoaded = function handleIframeLoaded() {
      log('Parent: Sending handshake');

      parent.addEventListener(MESSAGE, handleMessage);

      destructionPromise.then(function () {
        parent.removeEventListener(MESSAGE, handleMessage);
      });

      child.postMessage({
        penpal: HANDSHAKE,
        methodNames: Object.keys(methods)
      }, childOrigin);
    };

    iframe.addEventListener(LOAD, handleIframeLoaded);

    destructionPromise.then(function () {
      iframe.removeEventListener(LOAD, handleIframeLoaded);
      reject('Parent: Connection destroyed');
    });

    log('Parent: Loading iframe');

    iframe.src = url;
  });

  return {
    promise: promise,
    iframe: iframe,
    destroy: destroy,
    manualHandshake: manualHandshake
  };
};

Then manually call:

    window.addEventListener('message', (event) => {
      if (event.data.someDevMessageThatIndicatesLoad) {
        _connection.manualHandshake();
      }
    });

[michael-simon]

After a small bit of thought, we actually removed the manual handshake from the function return and made it a more general part of the Penpal library. (We also realized that maybe we should be triggering on module load, not on iframe load, for the asynchronous module loading case.) I made a PR to that end: #8

[Aaronius]

Interesting. A couple thoughts that come to mind:

(1) What if, instead of starting the handshake from the parent, we started it from the child. It's actually similar to what you're doing in your code, but rather than the child saying "hey parent, start the handshake", the child would be starting the handshake itself and waiting for the parent to acknowledge it. Maybe there's an obvious reason I'm missing as to why this wouldn't work. I need to look through the code to see what ramifications this would have.

(2) If option 1 doesn't work for some reason, we could instead retry the handshake on an interval until a response is received from the child.

Pros: It's resolved automatically. No need to send a message from the client, receive it from the parent, or call manualHandshake.
Cons: Waiting for an interval to end will probably take longer than the child telling the parent immediately when it's ready for the handshake. postMessage isn't too expensive though, so I think we could keep the intervals pretty short.

I can take a shot at implementation sometime. I would greatly prefer option 1. Thanks for reporting this issue. I do think it's something we should resolve.

[gajewsk2]

We both discussed. That makes a lot more sense. We'd prefer option 1 as well. 👍

[michael-simon]

That's a good thought, and it looks like the code supports that with one small problem (and it was already a problem for our thing too): what does the child use as the desired origin for the postMessage(HANDSHAKE call? Right now the only source of such information is a list of parentOrigins, but we're not allowed to pass in a list to postMessage. What my PR did was ask for a specific singular origin to use in the parameters of the reHandshakeWithParent function, but here we're inside of the connectToParent function which didn't take a specific singular origin.

What should ParentOrigin become to support this usecase? Once that's resolved, I could finish modifying my PR to match that model instead simply enough.

[Aaronius]

Yeah, you're right. We would probably need to limit parentOrigin to be a single string (right now it supports either a single string or an array of strings) and make it required (right now it's optional). I know the array of strings was useful to at least @Gomah because of #2

That's too bad. I don't really have other ideas for handling that. How do you feel about running the handshake on an interval (option 2 from my last comment)?

[michael-simon]

I don't love the interval idea.

What I want to know is what the use case for having multiple parentOrigins is. I can think of a couple cases:

1. You are attempting to connect a child to multiple 'parents'. This seems unlikely because of how the iframe is created by a singular parent in the first place. But you still have to post the message to some origin and that value is somewhat correct, so there has to be a 'right' value to use...
2. You are attempting to cover the parent having multiple origin aliases, where the 'correct' origin isn't always what's sent back because of weird reasons. In that case, it should really be a required parameter of 'parentOrigin' for the purpose of handshaking, and then a list of aliases/otherAllowedOrigins separately, right? This would work fine with our use case. If we wanted to support it as-stands without breaking the interface, we could say the singular or first parameter (if any) is the origin to try to handshake with setup-wise. Also, we could expose it to be called manually, in the case where parentOrigin is any of undefined, unclear or a list, instead of auto-calling it, if that makes people feel better.

[Aaronius]

The use case is that a company like Johnson & Johnson has multiple sites like http://tylenol.com and http://sudafed.com and they each need to load http://jnj.com/coupon.html into an iframe and communicate with it. In this case, from http://jnj.com/coupon.html they would provide Penpal an array of origins that includes both http://tylenol.com and http://sudafed.com. Does that make sense?

[gajewsk2]

If the parent is always initializing the child, then you could do this sequence to still support child handshake:

1. Parent makes iframe with get param including the origin
2. Child checks if get param contains origin
3. Child checks if selected origin is in it's array of valid origins
4. Child sends handshake back to parent at selected origin.

Would that be an acceptable solution to both problems?

[Aaronius]

It's an insecure approach. A nefarious website could pass an acceptable origin through a get param and start communicating with the iframe.

…

On Tue, Jul 18, 2017 at 10:24 PM Micah Gajewski ***@***.***> wrote: If the parent is always initializing the child, then you could do this sequence to still support child handshake: 1. Parent makes iframe with get param including the origin 2. Child checks if get param contains origin 3. Child checks if selected origin is in it's array of valid origins 4. Child sends handshake back to parent at selected origin. Would that be an acceptable solution to both problems? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAM3hJtcwjvsJJWBc_ma6klEXQPn4DrMks5sPYTtgaJpZM4Obtth> .

[Aaronius]

I spoke too soon. I don't think it will be insecure. I need to think about potential problems with adding to the URL though. For example, it may mess with consumers' analytics reporting. It may still be worth doing though.

…

On Tue, Jul 18, 2017 at 10:34 PM Aaron Hardy ***@***.***> wrote: It's an insecure approach. A nefarious website could pass an acceptable origin through a get param and start communicating with the iframe. On Tue, Jul 18, 2017 at 10:24 PM Micah Gajewski ***@***.***> wrote: > If the parent is always initializing the child, then you could do this > sequence to still support child handshake: > > 1. Parent makes iframe with get param including the origin > 2. Child checks if get param contains origin > 3. Child checks if selected origin is in it's array of valid origins > 4. Child sends handshake back to parent at selected origin. > > Would that be an acceptable solution to both problems? > > — > You are receiving this because you commented. > > > Reply to this email directly, view it on GitHub > <#7 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AAM3hJtcwjvsJJWBc_ma6klEXQPn4DrMks5sPYTtgaJpZM4Obtth> > . >

[Aaronius]

Okay, rather than having the parent include the origin on a get param, the child should just be able to use document.referrer to derive the parent's origin. It should be noted that if the child navigates itself to some other URL before attempting to handshake with the parent, then document.referrer will no longer refer to the parent's URL. I'm not too concerned about this case though because it's not supported even in the current Penpal.

Does this make sense to you? Feel free to give it a shot if you agree.

[Aaronius]

Fixed in 2.6.0. The child now initiates the handshake.

[Aaronius]

@gajewsk2 Did this work out okay for your system.js use case?