Skip to content

Commit

Permalink
Merge pull request #793 from cramforce/iframe-uris
Browse files Browse the repository at this point in the history
Allow iframes to be loaded from data URIs.
  • Loading branch information
cramforce committed Oct 30, 2015
2 parents 36d7cc7 + 65a363b commit 482034f
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 7 deletions.
28 changes: 26 additions & 2 deletions extensions/amp-iframe/0.1/amp-iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ class AmpIframe extends AMP.BaseElement {
var url = parseUrl(src);
assert(
url.protocol == 'https:' ||
url.protocol == 'data:' ||
url.origin.indexOf('http://iframe.localhost:') == 0,
'Invalid <amp-iframe> src. Must start with https://. Found %s',
this.element);
var containerUrl = parseUrl(containerSrc);
assert(
!((' ' + sandbox + ' ').match(/\s+allow-same-origin\s+/)) ||
url.origin != containerUrl.origin,
'Origin of <amp-iframe> must not be equal to container %s.',
'Origin of <amp-iframe> must not be equal to container %s' +
'if allow-same-origin is set.',
this.element);
return src;
}
Expand All @@ -61,9 +63,31 @@ class AmpIframe extends AMP.BaseElement {
minTop);
}

/**
* Transforms the srcdoc attribute if present to an equivalent data URI.
*
* It may be OK to change this later to leave the `srcdoc` in place and
* instead ensure that `allow-same-origin` is not present, but this
* implementation has the right security behavior which is that the document
* may under no circumstances be able to run JS on the parent.
* @return {string} Data URI for the srcdoc
*/
transformSrcDoc() {
var srcdoc = this.element.getAttribute('srcdoc');
if (!srcdoc) {
return;
}
var sandbox = this.element.getAttribute('sandbox');
assert(
!((' ' + sandbox + ' ').match(/\s+allow-same-origin\s+/)),
'allow-same-origin is not allowed with the srcdoc attribute %s.',
this.element);
return 'data:text/html;charset=utf-8;base64,' + btoa(srcdoc);
}

/** @override */
firstAttachedCallback() {
var iframeSrc = this.element.getAttribute('src');
var iframeSrc = this.element.getAttribute('src') || this.transformSrcDoc();
this.iframeSrc = this.assertSource(iframeSrc, window.location.href,
this.element.getAttribute('sandbox'));
this.preconnect.url(this.iframeSrc);
Expand Down
58 changes: 58 additions & 0 deletions extensions/amp-iframe/0.1/test/test-amp-iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,58 @@ describe('amp-iframe', () => {
});
});

it('should allow data-uri', () => {
var dataUri = 'data:text/html;charset=utf-8;base64,' +
'PHNjcmlwdD5kb2N1bWVudC53cml0ZSgnUiAnICsgZG9jdW1lbnQucmVmZXJyZXIgK' +
'yAnLCAnICsgbG9jYXRpb24uaHJlZik8L3NjcmlwdD4=';
return getAmpIframe({
src: dataUri,
width: 100,
height: 100
}).then(amp => {
expect(amp.iframe).to.be.instanceof(Element);
expect(amp.iframe.src).to.equal(dataUri);
expect(amp.iframe.getAttribute('sandbox')).to.equal('');
expect(amp.iframe.parentNode).to.equal(amp.scrollWrapper);
return timer.promise(0).then(() => {
expect(ranJs).to.equal(0);
});
});
});

it('should support srcdoc', () => {
return getAmpIframe({
width: 100,
height: 100,
sandbox: 'allow-scripts',
srcdoc: '<script>try{parent.location.href}catch(e){' +
'parent.parent./*OK*/postMessage(\'loaded-iframe\', \'*\');}' +
'</script>',
}).then(amp => {
expect(amp.iframe).to.be.instanceof(Element);
expect(amp.iframe.src).to.match(
/^data\:text\/html;charset=utf-8;base64,/);
expect(amp.iframe.getAttribute('srcdoc')).to.be.null;
expect(amp.iframe.getAttribute('sandbox')).to.equal(
'allow-scripts');
expect(amp.iframe.parentNode).to.equal(amp.scrollWrapper);
return timer.promise(0).then(() => {
expect(ranJs).to.equal(1);
});
});
});

it('should deny srcdoc with allow-same-origin', () => {
return getAmpIframe({
width: 100,
height: 100,
sandbox: 'allow-same-origin',
srcdoc: '',
}).then(amp => {
expect(amp.iframe).to.be.null;
});
});

it('should deny same origin', () => {
return getAmpIframeObject().then(amp => {
expect(() => {
Expand All @@ -182,6 +234,12 @@ describe('amp-iframe', () => {
amp.assertSource('http://iframe.localhost:123/foo',
'https://foo.com', '');
amp.assertSource('https://container.com', 'https://foo.com', '');

amp.element.setAttribute('srcdoc', 'abc');
amp.element.setAttribute('sandbox', 'allow-same-origin');
expect(() => {
amp.transformSrcDoc();
}).to.throw(/allow-same-origin is not allowed with the srcdoc attribute/);
});
});

Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-iframe/amp-iframe.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Displays an iframe.

- `amp-iframe` may not appear close to the top of the document. They must be either 600px away from the top or not within the first 75% of the viewport when scrolled to the top – whichever is smaller. NOTE: We are currently looking for feedback as to how well this restriction works in practice.
- They are sandboxed by default. That means that authors needs to be explicit about what should be allowed in the iframe. See the [the docs on MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe) for details on the sandbox attribute.
- They must only request resources via HTTPS.
- They must only request resources via HTTPS or from a data-URI or via the srcdoc attribute.
- They must not be in the same origin as the container unless they do not allow `allow-same-origin` in the sandbox attribute.

Example:
Expand All @@ -37,6 +37,6 @@ Example:

#### Attributes

**src, sandbox, frameborder, allowfullscreen, allowtransparency**
**src, srcdoc, sandbox, frameborder, allowfullscreen, allowtransparency**

The attributes above should all behave like they do on standard iframes.
13 changes: 11 additions & 2 deletions src/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ export function parseUrl(url) {
search: a.search,
hash: a.hash
};
info.origin = a.origin || getOrigin(info);
// For data URI a.origin is equal to the string 'null' which is not useful.
// We instead return the actual origin which is the full URL.
info.origin = (a.origin && a.origin != 'null') ? a.origin : getOrigin(info);
assert(info.origin, 'Origin must exist');
return info;
}
Expand Down Expand Up @@ -101,10 +103,17 @@ export function parseQueryString(queryString) {


/**
* Don't use this directly, only exported for testing. The value
* is available via the origin property of the object returned by
* parseUrl.
* @param {!Location} info
* @return {string}
* @visibleForTesting
*/
function getOrigin(info) {
export function getOrigin(info) {
if (info.protocol == 'data:' || !info.host) {
return info.href;
}
return info.protocol + '//' + info.host;
}

Expand Down
18 changes: 17 additions & 1 deletion test/functional/test-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {assertHttpsUrl, parseQueryString, parseUrl, removeFragment}
import {assertHttpsUrl, parseQueryString, parseUrl, removeFragment, getOrigin}
from '../../src/url';

describe('url', () => {
Expand Down Expand Up @@ -204,3 +204,19 @@ describe('removeFragment', () => {
'https://twitter.com/path');
});
});

describe('getOrigin', () => {
it('should parse https://twitter.com/path#abc', () => {
expect(getOrigin(parseUrl('https://twitter.com/path#abc')))
.to.equal('https://twitter.com');
expect(parseUrl('https://twitter.com/path#abc').origin)
.to.equal('https://twitter.com');
});

it('should parse data:12345', () => {
expect(getOrigin(parseUrl('data:12345')))
.to.equal('data:12345');
expect(parseUrl('data:12345').origin)
.to.equal('data:12345');
});
});

0 comments on commit 482034f

Please sign in to comment.