Aaronius / penpal

A promise-based library for securely communicating with iframes via postMessage.
MIT License
389 stars 56 forks source link

IE11 compatibility #1

Closed dozed closed 7 years ago

dozed commented 7 years ago

The library is currently not compatible with IE11.

There are two issues:

Possible URI fix var childOrigin = new URI(window.location.href).path(url).origin(); with https://medialize.github.io/URI.js/

A way for postMessage IE compatibility:

let Compat = {

  init: () => {
    var ieVersion = Compat.detectIE();
    Compat.stringifyPostMessages = (ieVersion !== false && Compat.detectIE() <= 11);
  },

  detectIE: () => {
    var ua = window.navigator.userAgent;

    var msie = ua.indexOf('MSIE ');
    if (msie > 0) {
      // IE 10 or older => return version number
      return parseInt(ua.substring(msie + 5, ua.indexOf('.', msie)), 10);
    }

    var trident = ua.indexOf('Trident/');
    if (trident > 0) {
      // IE 11 => return version number
      var rv = ua.indexOf('rv:');
      return parseInt(ua.substring(rv + 3, ua.indexOf('.', rv)), 10);
    }

    var edge = ua.indexOf('Edge/');
    if (edge > 0) {
      // Edge (IE 12+) => return version number
      return parseInt(ua.substring(edge + 5, ua.indexOf('.', edge)), 10);
    }

    // other browser
    return false;
  },

  postMessage: (target, data, remoteOrigin) => {

    if (Compat.stringifyPostMessages) {
      return Compat.postMessageCompat(target, data, remoteOrigin);
    } else {
      target.postMessage(data, remoteOrigin);
    }

  },

  addMessageListener: (target, listener) => {
    if (Compat.stringifyPostMessages) {
      return Compat.addMessageListenerCompat(target, listener);
    } else {
      target.addEventListener(MESSAGE, listener, false);

      return {
        remove: () => {
          target.removeEventListener(MESSAGE, listener);
        }
      };
    }
  },

  postMessageCompat: (target, data, remoteOrigin) => {

    let str = JSON.stringify(data);
    target.postMessage(str, remoteOrigin);

  },

  addMessageListenerCompat: (target, listener) => {

    let wrapped = (e) => {

      if (e.data && e.data.startsWith("{")) {
        let data = JSON.parse(e.data);
        let ec = {
          data: data,
          source: e.source,
          origin: e.origin
        };
        listener(ec);
      } else {
        listener(e);
      }

    };

    target.addEventListener(MESSAGE, wrapped);

    return {
      remove: () => {
        target.removeEventListener(MESSAGE, wrapped);
      }
    };

  }

};

Compat.init();
Aaronius commented 7 years ago

Thanks for reporting! I wasn't aware of these issues. I'll try to get them fixed as soon as possible.

Aaronius commented 7 years ago

@dozed Are you sure you aren't running IE in compatibility mode? I'm pretty sure IE9 and older required a string payload but I think everything after that supports it. My tests pass on IE11 and I can't reproduce the problems you're seeing. Can you provide me with a test or some specific code that is failing? Thanks.

dozed commented 7 years ago

You are right, IE10 and IE11 allow non-string payloads. So only IE9 and below are affected. Do you want to support those?

The issue with the URL appears in IE10/11 as well as in Edge, event.origin === childOrigin fails. Here is a test case: http://output.jsbin.com/yebucecibo Would it make sense to discuss this in a separate ticket?

Source jsbin:

Aaronius commented 7 years ago

I don't intend to support IE9 and below, but I do need to document what the supported browsers are. Thanks so much for creating the example jsbin. I'll take a closer look when I get a moment. No need to create a new ticket.

dozed commented 7 years ago

When providing url: http://output.jsbin.com/vovudekosi the childOrigin is http://output.jsbin.com/vovudekosi:80.

Aaronius commented 7 years ago

@dozed I think you're referring to childOrigin at the end of these lines:

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

If I put console.log(childOrigin); after those lines and run it, I see this in the browser console:

http://output.jsbin.com:80

Which is accurate. You should be able to see that here: http://jsbin.com/wenosinoxe/1/edit?html,js,output

Do you see something different?

dozed commented 7 years ago

Can you check if the handshake succeeds or if you can invoke a method on the parent? Also check the value of event.origin.

Aaronius commented 7 years ago

It doesn't succeed, but I wouldn't expect it to in the context of jsbin. The URL http://output.jsbin.com/vovudekosi, which is what the parent is trying to load, redirects to http://jsbin.com/vovudekosi/1/edit?html,js,output, which is jsbin itself and not the html page that has the connectToParent call. I think you'd have to pull this code out of jsbin to successfully test the full flow.

dozed commented 7 years ago

This is since the jsbin is expired. You would need to setup a new jsbin with the code to reproduce the issue with the URL.

Aaronius commented 7 years ago

Okay, I think I have this figured out and fixed in 2.4.1. You can see my changes here: https://github.com/Aaronius/penpal/commit/f04c4907be4e513fea0e78c1f5e18ceae4c4118c

Basically, in browsers where we have to compute the childOrigin, we need to not add the port to the computed string if the port is the default for the protocol. Because we were always adding the port, it wasn't matching the origin reported by the browser on message events.

The approach I've taken appears to match what's done in the whatwg-url library here.

You might be able to view the working jsbin here: http://output.jsbin.com/leqovofite

Thanks again for reporting this issue and writing up the jsbin. Please let me know if you see any problems.

dozed commented 7 years ago

On Microsoft Edge a.port || location.port is the empty string. So it does not work there.

Aaronius commented 7 years ago

:/

Aaronius commented 7 years ago

Fixed in 2.4.3. Currently viewable at: http://output.jsbin.com/jucovogugi/1

I tested in IE 10 and 11, Edge, and latest versions of Chrome, Firefox, and Safari. My automated tests don't use the default ports or it would have caught this. I don't really want to use the default ports though because it may not catch the problems with custom ports. I might need to run them on both default and custom ports. :(

Thanks for testing this for me.

dozed commented 7 years ago

Cool, thanks for fixing.

Aaronius commented 7 years ago

@dozed Out of curiosity, are you actively using Penpal or just checking it out? If you're using it, how is it working out for you?

dozed commented 7 years ago

I use it actively in a private project, it is working fine so far. I like the simple functional approach.

Aaronius commented 7 years ago

Good to hear. Thanks!