Aaronius / penpal

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

communications fail on IE Edge with srcdoc polyfill #29

Closed CNDW closed 5 years ago

CNDW commented 5 years ago

There is currently an issue with the srcdoc attribute on IE edge, there is a workaround detailed in this pr for the srcdoc polyfill.

I am using penpal with react to insert an iframe with srcdoc content with the following config

        this.iframeEl = document.createElement('iframe');
        this.iframeEl.setAttribute('sandbox', 'allow-same-origin allow-scripts');

        this.iframeEl.setAttribute('srcdoc', iframeContent);
        if (needsPolyfill) {
            this.iframeEl.setAttribute('src', 'javascript: window.frameElement.getAttribute(\'srcDoc\');');
        }
        this.iframeEl.onload = this.onLoad;

        this.connection = Penpal.connectToChild({
            url: window.location.origin,
            iframe: this.iframeEl,
            methods: {
                  ...
            },
        });

It is working as expected when the polyfill is not needed, but in IE Edge where the polyfill is needed, penpal clobbers over the src attribute I have assigned and the polyfill fails to work.

If I work around it by using the technique detailed in the issue above, penpal fails to complete the handshake process here because event.origin no longer matches childOrigin

penpal should respect the src attribute if given an iframe element, or there should be some way for me to define what I expect the event.origin to be when the handshake process initializes (I could see this second point being an issue if there is some sort of oauth process in the iframe that causes the frame to end at a different place than it was initialized). As a nuclear option, it would be nice to be able to opt out of that specific layer of security if I am using srcdoc because I am in control of the content in the iframe.

Aaronius commented 5 years ago

Hey @CNDW. Just wanted to let you know I saw your issue. I haven't investigated usage of Penpal with srcdoc yet, so this will be interesting. I'll try to get some time soon to dig into the issue.

Aaronius commented 5 years ago

Could you give me the value of iframeContent in your example? Feel free to trim out the irrelevant bits.

Aaronius commented 5 years ago

Also, I know Penpal will stomp over src currently, but if it didn't, would you be able to use a data URI e.g., (data:text/html,<html>...</html>) instead of a JavaScript scheme?

CNDW commented 5 years ago

iframeContent is a document string in that example. e.g.

<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8">
        <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
        <meta name="viewport" content="width=device-width,initial-scale=1">
        <title>title</title>
    </head>
    <body style="margin: 0;">
    </body>
</html>

The javascript scheme is the polyfill, I'm unsure of what kind of support a data URI would have but as long as I am able to support the same browsers with a data uri instead of a javascript scheme it would work.

Aaronius commented 5 years ago

@CNDW, I just released version 4 of Penpal which includes both srcdoc support and data URI support when using src. In your case, you would set srcdoc in browsers that support it and instead set src in browsers that don't. When setting src, you would use a data URI as follows:

iframe.src = `data:text/html,${htmlSrc}`;

I decided not to support JavaScript schemes, because Edge reports a different origin on postMessage messages than all the other supported browsers and I'd prefer to not have browser detection code and the accompanying workaround in Penpal if possible. It seems like using a data URI should be a reasonable alternative.

Note that version 4 of Penpal drops Internet Explorer support.

Let me know how it goes!

CNDW commented 5 years ago

Thank you for the work on this.

Note that version 4 of Penpal drops Internet Explorer support.

This makes it a dealbreaker for me, I still have to support IE 11. At this point I will likely have to fork an older version and make a manual fix :(