Navigation Menu

Skip to content

Commit

Permalink
馃悰 Remove oldschool HTML attribrutes (#7309)
Browse files Browse the repository at this point in the history 
no issue

Uses `allowedAttributes` functionality of `Sanitize` HTML and whitelists attributes for certain tags, regarding
AMP validation rules.

This PR fixes issues with inline style like `border`, `bgcolor`, `align` and so on.
  • Loading branch information
@aileen @kirrg001
aileen authored and kirrg001 committed Sep 1, 2016
1 parent c80f481 commit 1143631
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 18 deletions.
99 changes: 83 additions & 16 deletions core/server/apps/amp/lib/helpers/amp_content.js
View file
Expand Up @@ -17,6 +17,7 @@ var hbs = require('express-hbs'),
amperize = new Amperize(),
amperizeCache = {},
allowedAMPTags = [],
allowedAMPAttributes = {},
cleanHTML,
ampHTML;

Expand All @@ -30,13 +31,86 @@ allowedAMPTags = ['html', 'body', 'article', 'section', 'nav', 'aside', 'h1', 'h
'path', 'glyph', 'glyphref', 'marker', 'view', 'circle', 'line', 'polygon',
'polyline', 'rect', 'text', 'textpath', 'tref', 'tspan', 'clippath',
'filter', 'lineargradient', 'radialgradient', 'mask', 'pattern', 'vkern',
'hkern', 'defs', 'use', 'symbol', 'desc', 'title', 'table', 'caption',
'colgroup', 'col', 'tbody', 'thead', 'tfoot', 'tr', 'td', 'th', 'button',
'noscript', 'acronym', 'center', 'dir', 'hgroup', 'listing', 'multicol',
'nextid', 'nobr', 'spacer', 'strike', 'tt', 'xmp', 'amp-img', 'amp-video',
'amp-ad', 'amp-fit-text', 'amp-font', 'amp-carousel', 'amp-anim',
'amp-youtube', 'amp-twitter', 'amp-vine', 'amp-instagram', 'amp-iframe',
'amp-pixel', 'amp-audio', 'amp-lightbox', 'amp-image-lightbox'];
'hkern', 'defs', 'stop', 'use', 'foreignobject', 'symbol', 'desc', 'title',
'table', 'caption', 'colgroup', 'col', 'tbody', 'thead', 'tfoot', 'tr', 'td',
'th', 'button', 'noscript', 'acronym', 'center', 'dir', 'hgroup', 'listing',
'multicol', 'nextid', 'nobr', 'spacer', 'strike', 'tt', 'xmp', 'amp-img',
'amp-video', 'amp-ad', 'amp-embed', 'amp-anim', 'amp-iframe', 'amp-pixel',
'amp-audio', 'O:P'];

allowedAMPAttributes = {
'*': ['itemid', 'itemprop', 'itemref', 'itemscope', 'itemtype', 'accesskey', 'class', 'dir', 'draggable',
'id', 'lang', 'tabindex', 'title', 'translate', 'aria-*', 'role', 'placeholder', 'fallback', 'lightbox',
'overflow', 'amp-access', 'amp-access-*', 'i-amp-access-id'],
h1: ['align'],
h2: ['align'],
h3: ['align'],
h4: ['align'],
h5: ['align'],
h6: ['align'],
p: ['align'],
blockquote: ['align'],
ol: ['reversed', 'start', 'type'],
li: ['value'],
div: ['align'],
a: ['href', 'hreflang', 'rel', 'role', 'tabindex', 'target', 'download', 'media', 'type', 'border', 'name'],
time: ['datetime'],
bdo: ['dir'],
ins: ['datetime'],
del: ['datetime'],
source: ['src', 'srcset', 'sizes', 'media', 'type', 'kind', 'label', 'srclang'],
track: ['src', 'default', 'kind', 'label', 'srclang'],
svg: ['*'],
g: ['*'],
glyph: ['*'],
glyphref: ['*'],
marker: ['*'],
path: ['*'],
view: ['*'],
circle: ['*'],
line: ['*'],
polygon: ['*'],
polyline: ['*'],
rect: ['*'],
text: ['*'],
textpath: ['*'],
tref: ['*'],
tspan: ['*'],
clippath: ['*'],
filter: ['*'],
hkern: ['*'],
lineargradient: ['*'],
mask: ['*'],
pattern: ['*'],
radialgradient: ['*'],
stop: ['*'],
vkern: ['*'],
defs: ['*'],
symbol: ['*'],
use: ['*'],
foreignobject: ['*'],
desc: ['*'],
title: ['*'],
table: ['sortable', 'align', 'border', 'bgcolor', 'cellpadding', 'cellspacing', 'width'],
colgroup: ['span'],
col: ['span'],
tr: ['align', 'bgcolor', 'height', 'valign'],
td: ['align', 'bgcolor', 'height', 'valign', 'colspan', 'headers', 'rowspan'],
th: ['align', 'bgcolor', 'height', 'valign', 'colspan', 'headers', 'rowspan', 'abbr', 'scope', 'sorted'],
button: ['disabled', 'name', 'role', 'tabindex', 'type', 'value', 'formtarget'],
// built ins
'amp-img': ['media', 'noloading', 'alt', 'attribution', 'placeholder', 'src', 'srcset', 'width', 'height', 'layout'],
'amp-pixel': ['src'],
'amp-video': ['src', 'srcset', 'media', 'noloading', 'width', 'height', 'layout', 'alt', 'attribution',
'autoplay', 'controls', 'loop', 'muted', 'poster', 'preload'],
'amp-embed': ['media', 'noloading', 'width', 'height', 'layout', 'type', 'data-*', 'json'],
'amp-ad': ['media', 'noloading', 'width', 'height', 'layout', 'type', 'data-*', 'json'],
// extended components we support
'amp-anim': ['media', 'noloading', 'alt', 'attribution', 'placeholder', 'src', 'srcset', 'width', 'height', 'layout'],
'amp-audio': ['src', 'width', 'height', 'autoplay', 'loop', 'muted', 'controls'],
'amp-iframe': ['src', 'srcdoc', 'width', 'height', 'layout', 'frameborder', 'allowfullscreen', 'allowtransparency',
'sandbox', 'referrerpolicy']
};

function getAmperizeHTML(html, post) {
if (!html) {
Expand Down Expand Up @@ -81,21 +155,14 @@ function ampContent() {
// errors in video, because video will be stripped out.
// @TODO: remove this, when Amperize support video transform
$('video').children('source').remove();

// Vimeo iframe e. g. come with prohibited attributes
// @TODO: remove this, when Amperize supports HTML sanitizing
$('amp-iframe').removeAttr('webkitallowfullscreen');
$('amp-iframe').removeAttr('mozallowfullscreen');

// No inline style allowed
$('*').removeAttr('style');
$('video').children('track').remove();

ampHTML = $.html();

// @TODO: remove this, when Amperize supports HTML sanitizing
cleanHTML = sanitizeHtml(ampHTML, {
allowedTags: allowedAMPTags,
allowedAttributes: false,
allowedAttributes: allowedAMPAttributes,
selfClosing: ['source', 'track']
});

Expand Down
14 changes: 12 additions & 2 deletions core/server/apps/amp/tests/amp_content_spec.js
View file
Expand Up @@ -154,6 +154,7 @@ describe('{{amp_content}} helper', function () {
it('removes video tags including source children', function (done) {
var testData = {
html: '<video width="480" controls poster="https://archive.org/download/WebmVp8Vorbis/webmvp8.gif" >' +
'<track kind="captions" src="https://archive.org/download/WebmVp8Vorbis/webmvp8.webm" srclang="en">' +
'<source src="https://archive.org/download/WebmVp8Vorbis/webmvp8.webm" type="video/webm">' +
'<source src="https://archive.org/download/WebmVp8Vorbis/webmvp8_512kb.mp4" type="video/mp4">' +
'Your browser doesn\'t support HTML5 video tag.' +
Expand All @@ -173,11 +174,20 @@ describe('{{amp_content}} helper', function () {

it('removes inline style', function (done) {
var testData = {
html: '<amp-img src="/content/images/2016/08/aileen_small.jpg" style="border-radius: 50%" width="50" height="50" layout="responsive"></amp-img>',
html: '<amp-img src="/content/images/2016/08/aileen_small.jpg" style="border-radius: 50%"; !important' +
'border="0" align="center" font="Arial" width="50" height="50" layout="responsive"></amp-img>' +
'<p align="right" style="color: red; !important" bgcolor="white">Hello</p>' +
'<table style="width:100%"><tr bgcolor="tomato" colspan="2"><th font="Arial">Name:</th> ' +
'<td color="white" colspan="2">Bill Gates</td></tr><tr><th rowspan="2" valign="center">Telephone:</th> ' +
'<td>55577854</td></tr></table>',
updated_at: 'Wed Jul 27 2016 18:17:22 GMT+0200 (CEST)',
id: 1
},
expectedResult = '<amp-img src="https://my-awesome-blog.com/content/images/2016/08/aileen_small.jpg" width="50" height="50" layout="responsive"></amp-img>',
expectedResult = '<amp-img src="https://my-awesome-blog.com/content/images/2016/08/aileen_small.jpg" width="50" ' +
'height="50" layout="responsive"></amp-img><p align="right">Hello</p>' +
'<table><tr bgcolor="tomato"><th>Name:</th> ' +
'<td colspan="2">Bill Gates</td></tr><tr><th rowspan="2" valign="center">Telephone:</th> ' +
'<td>55577854</td></tr></table>',
ampResult = ampContentHelper.call(testData);

ampResult.then(function (rendered) {
Expand Down

0 comments on commit 1143631

Please sign in to comment.