Skip to content

Sandboxing: Adding disclaimers about `postMessage` + cleanup. #263

Merged
merged 4 commits into from Jan 9, 2013

3 participants

@mikewest
HTML5Rocks member
mikewest commented Jan 8, 2013
  1. postMessage can be dangerous if you're not careful; the demo code in
    this article didn't make that clear. This patch adds some comments to
    ensure that the important points are clearly highlighted, and adjusts
    the demo code itself to actually do rudimentary validation of sources
    rather than blindly executing code in response to a message event.

  2. Cleanup in the formatting of some code at the bottom of the article.
    One space too many, ah well.

@mikewest mikewest Sandboxing: Adding disclaimers about `postMessage` + cleanup.
1.  `postMessage` can be dangerous if you're not careful; the demo code in
    this article didn't make that clear. This patch adds some comments to
    ensure that the important points are clearly highlighted, and adjusts
    the demo code itself to actually do rudimentary validation of sources
    rather than blindly executing code in response to a message event.

2.  Cleanup in the formatting of some code at the bottom of the article.
    One space too many, ah well.
884a4c2
@mikewest
HTML5Rocks member
mikewest commented Jan 8, 2013

@abarth: Would you mind taking a look at this change as well? If there's more I can do to ensure that the demo code isn't leading folks down the wrong path, I'm happy to make more changes.

@abarth

The source is actually the DOMWindow of the frame, so you'll need to write something like:

if (e.origin === "null" && e.source === frame.contentWindow)

HTML5Rocks member

Ugh. facepalm. Yes. Thanks. :)

@abarth

Consider using window.location.origin

Similarly, you'll need unsandboxedFrame.contentWindow (assuming unsandboxedFrame points to the element rather than the frame's DOMWindow).

HTML5Rocks member

window.location.origin doesn't seem to work in Gecko. This is pretty ugly, but was the closest thing to cross-platform I could come up with.

Yes, I think it's only implemented in WebKit. What you have now probably works fine.

I wonder if we should write a patch for Firefox that implements location.origin. It's part of the URL spec: http://url.spec.whatwg.org/#dom-url-origin

HTML5Rocks member

I was looking for a bug yesterday, filed https://bugzilla.mozilla.org/show_bug.cgi?id=828261 today. Might be a nice way of getting my feet wet in Firefox. :)

@abarth
abarth commented Jan 8, 2013

I added some comments to the patch.

mikewest added some commits Jan 8, 2013
@mikewest mikewest @abarth's feedback.
* e.origin === sandboxedFrame.contentWindow
049c014
@mikewest mikewest Update content/tutorials/security/sandboxed-iframes/en/index.html
e.origin === frame.contentWindow.
b0736c3
@mikewest mikewest Update content/tutorials/security/sandboxed-iframes/en/index.markdown
e.source === frame.contentWindow
889bc56
@mikewest
HTML5Rocks member
mikewest commented Jan 8, 2013

Thanks Adam, much appreciated.

@cwilso
cwilso commented Jan 8, 2013

If this is ready, I'll pull and deploy it tomorrow.

@mikewest
HTML5Rocks member
mikewest commented Jan 9, 2013

I'd appreciate you pulling it, Chris. I think it's good to go, and it's certainly better than what's out there now. :)

@cwilso cwilso merged commit 7e0c266 into html5rocks:master Jan 9, 2013
@cwilso
cwilso commented Jan 9, 2013

Deployed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.