Skip to content

Commit

Permalink
Bug 922577 - Do not leak attributes between translations (#73)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stasm
stasm committed Sep 29, 2017
1 parent e49e237 commit 64ef212
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 53 deletions.
96 changes: 64 additions & 32 deletions fluent-dom/src/overlay.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ const reOverlay = /<|&#?\w+;/;
*
* Source: https://www.w3.org/TR/html5/text-level-semantics.html
*/
const ALLOWED_ELEMENTS = {
const LOCALIZABLE_ELEMENTS = {
'http://www.w3.org/1999/xhtml': [
'a', 'em', 'strong', 'small', 's', 'cite', 'q', 'dfn', 'abbr', 'data',
'time', 'code', 'var', 'samp', 'kbd', 'sub', 'sup', 'i', 'b', 'u',
'mark', 'ruby', 'rt', 'rp', 'bdi', 'bdo', 'span', 'br', 'wbr'
],
};

const ALLOWED_ATTRIBUTES = {
const LOCALIZABLE_ATTRIBUTES = {
'http://www.w3.org/1999/xhtml': {
global: ['title', 'aria-label', 'aria-valuetext', 'aria-moz-hint'],
a: ['download'],
area: ['download', 'alt'],
// value is special-cased in isAttrNameAllowed
// value is special-cased in isAttrNameLocalizable
input: ['alt', 'placeholder'],
menuitem: ['label'],
menu: ['label'],
Expand Down Expand Up @@ -69,18 +69,24 @@ export default function overlayElement(targetElement, translation) {
}
}

if (translation.attrs === null) {
return;
}

const explicitlyAllowed = targetElement.hasAttribute('data-l10n-attrs')
? targetElement.getAttribute('data-l10n-attrs')
.split(',').map(i => i.trim())
: null;

for (const [name, val] of translation.attrs) {
if (isAttrNameAllowed(name, targetElement, explicitlyAllowed)) {
targetElement.setAttribute(name, val);
// Remove localizable attributes which may have been set by a previous
// translation.
for (const attr of Array.from(targetElement.attributes)) {
if (isAttrNameLocalizable(attr.name, targetElement, explicitlyAllowed)) {
targetElement.removeAttribute(attr.name);
}
}

if (translation.attrs) {
for (const [name, val] of translation.attrs) {
if (isAttrNameLocalizable(name, targetElement, explicitlyAllowed)) {
targetElement.setAttribute(name, val);
}
}
}
}
Expand Down Expand Up @@ -108,9 +114,11 @@ export default function overlayElement(targetElement, translation) {
*
* @param {DocumentFragment} translationFragment
* @param {Element} sourceElement
* @returns {DocumentFragment}
* @private
*/
function sanitizeUsing(translationFragment, sourceElement) {
const ownerDocument = translationFragment.ownerDocument;
// Take one node from translationFragment at a time and check it against
// the allowed list or try to match it with a corresponding element
// in the source.
Expand All @@ -121,32 +129,29 @@ function sanitizeUsing(translationFragment, sourceElement) {
}

// If the child is forbidden just take its textContent.
if (!isElementAllowed(childNode)) {
const text = translationFragment.ownerDocument.createTextNode(
childNode.textContent
);
if (!isElementLocalizable(childNode)) {
const text = ownerDocument.createTextNode(childNode.textContent);
translationFragment.replaceChild(text, childNode);
continue;
}


// If a child of the same type exists in sourceElement, use it as the base
// for the resultChild. This also removes the child from sourceElement.
const sourceChild = shiftNamedElement(sourceElement, childNode.localName);

const mergedChild = sourceChild
// Shallow-clone the sourceChild to remove all childNodes.
? sourceChild.cloneNode(false)
// Create a fresh element as a way to remove all forbidden attributes.
: childNode.ownerDocument.createElement(childNode.localName);
// Start the sanitization with an empty element.
const mergedChild = ownerDocument.createElement(childNode.localName);

// Explicitly discard nested HTML by serializing childNode to a TextNode.
mergedChild.textContent = childNode.textContent;

for (const attr of Array.from(childNode.attributes)) {
if (isAttrNameAllowed(attr.name, childNode)) {
mergedChild.setAttribute(attr.name, attr.value);
}
// If a child of the same type exists in sourceElement, take its functional
// (i.e. non-localizable) attributes. This also removes the child from
// sourceElement.
const sourceChild = shiftNamedElement(sourceElement, childNode.localName);

// Find the union of all safe attributes: localizable attributes from
// childNode and functional attributes from sourceChild.
const safeAttributes = sanitizeAttrsUsing(childNode, sourceChild);

for (const attr of safeAttributes) {
mergedChild.setAttribute(attr.name, attr.value);
}

translationFragment.replaceChild(mergedChild, childNode);
Expand All @@ -159,6 +164,33 @@ function sanitizeUsing(translationFragment, sourceElement) {
return translationFragment;
}

/**
* Sanitize and merge attributes.
*
* Only localizable attributes from the translated child element and only
* functional attributes from the source child element are considered safe.
*
* @param {Element} translatedElement
* @param {Element} sourceElement
* @returns {Array<Attr>}
* @private
*/
function sanitizeAttrsUsing(translatedElement, sourceElement) {
const localizedAttrs = Array.from(translatedElement.attributes).filter(
attr => isAttrNameLocalizable(attr.name, translatedElement)
);

if (!sourceElement) {
return localizedAttrs;
}

const functionalAttrs = Array.from(sourceElement.attributes).filter(
attr => !isAttrNameLocalizable(attr.name, sourceElement)
);

return localizedAttrs.concat(functionalAttrs);
}

/**
* Check if element is allowed in the translation.
*
Expand All @@ -169,8 +201,8 @@ function sanitizeUsing(translationFragment, sourceElement) {
* @returns {boolean}
* @private
*/
function isElementAllowed(element) {
const allowed = ALLOWED_ELEMENTS[element.namespaceURI];
function isElementLocalizable(element) {
const allowed = LOCALIZABLE_ELEMENTS[element.namespaceURI];
return allowed && allowed.includes(element.localName);
}

Expand All @@ -190,12 +222,12 @@ function isElementAllowed(element) {
* @returns {boolean}
* @private
*/
function isAttrNameAllowed(name, element, explicitlyAllowed = null) {
function isAttrNameLocalizable(name, element, explicitlyAllowed = null) {
if (explicitlyAllowed && explicitlyAllowed.includes(name)) {
return true;
}

const allowed = ALLOWED_ATTRIBUTES[element.namespaceURI];
const allowed = LOCALIZABLE_ATTRIBUTES[element.namespaceURI];
if (!allowed) {
return false;
}
Expand Down
94 changes: 73 additions & 21 deletions fluent-dom/test/overlay_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,22 @@ suite('Filtering attributes in translation', function() {
assert.equal(element.innerHTML,
'FOO <a title="BAR">BAR</a>');
});

test('attributes of source children do not leak', function() {
const element = elem('div')`
<a href="foo" title="Foo">Foo</a>`;
const translation = {
value: '<a>FOO</a>',
attrs: null
};

overlayElement(element, translation);
assert.equal(element.innerHTML,
'<a href="foo">FOO</a>');
});
});

suite('Filtering attributes on the main elment', function() {
suite('Filtering top-level attributes', function() {
test('allowed attribute', function() {
const element = elem('div')``;
const translation = {
Expand All @@ -108,10 +121,23 @@ suite('Filtering attributes on the main elment', function() {
overlayElement(element, translation);
assert.equal(element.outerHTML, '<input>');
});

test('attributes of the source element do not leak', function() {
const element = elem('div')`Foo`;
element.setAttribute('title', 'Title');

const translation = {
value: 'FOO',
attrs: null
};

overlayElement(element, translation);
assert.equal(element.outerHTML,
'<div>FOO</div>');
});
});

suite('Overlay DOM elements', function() {

test('string value', function() {
const element = elem('div')`Foo`;
const translation = {
Expand All @@ -133,7 +159,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'FOO <a href="bar" title="BAZ">BAZ</a>');
'FOO <a title="BAZ" href="bar">BAZ</a>');
});

test('one child with own attributes', function() {
Expand All @@ -146,7 +172,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'FOO <a href="bar" title="BAZ">BAZ</a>');
'FOO <a title="BAZ" href="bar">BAZ</a>');
});

test('two children of the same type', function() {
Expand All @@ -159,7 +185,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'<a href="foo" title="FOO">FOO</a> <a href="bar" title="BAR">BAR</a>');
'<a title="FOO" href="foo">FOO</a> <a title="BAR" href="bar">BAR</a>');
});

test('two children of different types in order', function() {
Expand All @@ -172,7 +198,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'<a href="foo" title="FOO">FOO</a> <em class="bar">BAR</em>');
'<a title="FOO" href="foo">FOO</a> <em class="bar">BAR</em>');
});

test('two children of different types not in order', function() {
Expand All @@ -185,7 +211,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'<em class="bar">BAR</em> <a href="foo" title="FOO">FOO</a>');
'<em class="bar">BAR</em> <a title="FOO" href="foo">FOO</a>');
});

test('HTML missing in translation', function() {
Expand All @@ -211,7 +237,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'FOO <a href="bar" title="BAZ">BAZ</a>');
'FOO <a title="BAZ" href="bar">BAZ</a>');
});

test('nested HTML in translation', function() {
Expand All @@ -224,7 +250,7 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'FOO <a href="bar" title="BAZ">BAZ</a>');
'FOO <a title="BAZ" href="bar">BAZ</a>');
});

test('nested HTML in both', function() {
Expand All @@ -237,13 +263,13 @@ suite('Overlay DOM elements', function() {

overlayElement(element, translation);
assert.equal(element.innerHTML,
'FOO <a href="bar" title="BAZ">BAZ</a>');
'FOO <a title="BAZ" href="bar">BAZ</a>');
});
});

suite('Retranslation', function() {
// XXX https://bugzilla.mozilla.org/show_bug.cgi?id=922577
test('leaking attribute', function() {
test('top-level attributes do not leak', function() {
const element = elem('div')`Foo`;
const translationA = {
value: 'FOO A',
Expand All @@ -261,19 +287,45 @@ suite('Retranslation', function() {
'<div title="TITLE A">FOO A</div>');
overlayElement(element, translationB);
assert.equal(element.outerHTML,
'<div title="TITLE A">FOO B</div>');
'<div>FOO B</div>');
});

test('forbidden attribute', function() {
const element = elem('input')``;
const translation = {
value: null,
attrs: [
['disabled', 'DISABLED']
]
test('attributes of children do not leak', function() {
const element = elem('div')`
<a href="foo">Foo</a>`;
const translationA = {
value: '<a title="FOO A">FOO A</em>',
attrs: null
};
const translationB = {
value: '<a>FOO B</em>',
attrs: null
};

overlayElement(element, translation);
assert.equal(element.outerHTML, '<input>');
overlayElement(element, translationA);
assert.equal(element.innerHTML,
'<a title="FOO A" href="foo">FOO A</a>');
overlayElement(element, translationB);
assert.equal(element.innerHTML,
'<a href="foo">FOO B</a>');
});

test('attributes of inserted children do not leak', function() {
const element = elem('div')`Foo`;
const translationA = {
value: 'FOO <em title="A">A</em>',
attrs: null
};
const translationB = {
value: 'FOO <em>B</em>',
attrs: null
};

overlayElement(element, translationA);
assert.equal(element.innerHTML,
'FOO <em title="A">A</em>');
overlayElement(element, translationB);
assert.equal(element.innerHTML,
'FOO <em>B</em>');
});
});

0 comments on commit 64ef212

Please sign in to comment.