Skip to content

Commit

Permalink
fix 'Need to clean up 'sandbox', 'target' and 'style' attributes'(close
Browse files Browse the repository at this point in the history
#1448) (#1450)

* fix 'Need to clean up 'sandbox', 'target', 'style' attributes'(close #1448)

* fix tests
  • Loading branch information
miherlosev authored and LavrovArtem committed Jan 17, 2018
1 parent 2d30ccd commit 293d5fa
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/client/sandbox/node/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import * as windowsStorage from '../windows-storage';
import AttributesWrapper from '../code-instrumentation/properties/attributes-wrapper';
import ShadowUI from '../shadow-ui';
import DOMMutationTracker from './live-node-list/dom-mutation-tracker';
import { ATTRS_WITH_SPECIAL_PROXYING_LOGIC } from '../../../processing/dom/attributes';

const KEYWORD_TARGETS = ['_blank', '_self', '_parent', '_top'];
const ATTRS_WITH_SPECIAL_PROXYING_LOGIC = ['sandbox', 'autocomplete', 'target', 'style'];
const KEYWORD_TARGETS = ['_blank', '_self', '_parent', '_top'];

// NOTE: We should avoid using native object prototype methods,
// since they can be overriden by the client code. (GH-245)
Expand Down
3 changes: 2 additions & 1 deletion src/client/sandbox/node/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import domProcessor from '../../dom-processor';
import * as domUtils from '../../utils/dom';
import { getNativeQuerySelectorAll } from '../../utils/query-selector';
import nativeMethods from '../native-methods';
import { URL_ATTRS } from '../../../processing/dom/attributes';

const ATTRIBUTE_SELECTOR_REG_EX = /\[([\w-]+)(\^?=.+?)]/g;
const ATTRIBUTE_OPERATOR_WITH_HASH_VALUE = /^\W+\s*#/;
Expand Down Expand Up @@ -145,7 +146,7 @@ export default class NodeSandbox extends SandboxBase {
return selector;

return selector.replace(ATTRIBUTE_SELECTOR_REG_EX, (str, name, operatorWithValue) => {
if (domProcessor.URL_ATTRS.indexOf(name) !== -1 &&
if (URL_ATTRS.indexOf(name) !== -1 &&
!ATTRIBUTE_OPERATOR_WITH_HASH_VALUE.test(operatorWithValue)) {
name = getStoredAttrName(name);

Expand Down
19 changes: 16 additions & 3 deletions src/client/utils/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { convertToProxyUrl, parseProxyUrl } from './url';
import { isIE, isMSEdge } from './browser';
import * as urlResolver from './url-resolver';
import INTERNAL_PROPS from '../../processing/dom/internal-properties';
import { URL_ATTRS, ATTRS_WITH_SPECIAL_PROXYING_LOGIC } from '../../processing/dom/attributes';

const FAKE_TAG_NAME_PREFIX = 'hh_fake_tag_name_';
const FAKE_DOCTYPE_TAG_NAME = 'hh_fake_doctype';
Expand All @@ -30,11 +31,23 @@ const UNWRAP_DOCTYPE_RE = new RegExp(`<${ FAKE_DOCTYPE_TAG_NAME }>([\\S\\s]*
const FIND_SVG_RE = /<svg\s?[^>]*>/ig;
const FIND_NS_ATTRS_RE = /\s(?:NS[0-9]+:[^"']+('|")[\S\s]*?\1|[^:]+:NS[0-9]+=(?:""|''))/g;

const ATTRS_FOR_CLEANING = DomProcessor.getAttrsForCleaning();
// NOTE: We should avoid using native object prototype methods,
// since they can be overriden by the client code. (GH-245)
const arrayConcat = Array.prototype.concat;
const arrayMap = Array.prototype.map;

const ATTRS_FOR_CLEANING = arrayConcat.call(URL_ATTRS, ATTRS_WITH_SPECIAL_PROXYING_LOGIC);
const ATTRS_DATA_FOR_CLEANING = arrayMap.call(ATTRS_FOR_CLEANING, attr => {
return {
attr,
storedAttr: DomProcessor.getStoredAttrName(attr)
};
});

const STORED_ATTRS_SELECTOR = (() => {
const storedAttrs = [];

for (const { storedAttr } of ATTRS_FOR_CLEANING)
for (const { storedAttr } of ATTRS_DATA_FOR_CLEANING)
storedAttrs.push(storedAttr);

return '[' + storedAttrs.join('],[') + ']';
Expand Down Expand Up @@ -124,7 +137,7 @@ export function cleanUpHtml (html) {
let changed = false;

find(container, STORED_ATTRS_SELECTOR, el => {
for (const { attr, storedAttr } of ATTRS_FOR_CLEANING) {
for (const { attr, storedAttr } of ATTRS_DATA_FOR_CLEANING) {
if (el.hasAttribute(attr))
nativeMethods.setAttribute.call(el, attr, nativeMethods.getAttribute.call(el, storedAttr));
else if (attr === 'autocomplete')
Expand Down
12 changes: 12 additions & 0 deletions src/processing/dom/attributes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const URL_ATTR_TAGS = {
href: ['a', 'link', 'image', 'area', 'base'],
src: ['img', 'embed', 'script', 'source', 'video', 'audio', 'input', 'frame', 'iframe'],
action: ['form'],
formaction: ['button', 'input'],
manifest: ['html'],
data: ['object']
};

export const URL_ATTRS = Object.keys(URL_ATTR_TAGS);

export const ATTRS_WITH_SPECIAL_PROXYING_LOGIC = ['sandbox', 'autocomplete', 'target', 'style'];
24 changes: 1 addition & 23 deletions src/processing/dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { isScriptProcessed, processScript } from '../script';
import styleProcessor from '../../processing/style';
import * as urlUtils from '../../utils/url';
import { XML_NAMESPACE } from './namespaces';
import { URL_ATTR_TAGS, URL_ATTRS } from './attributes';

const CDATA_REG_EX = /^(\s)*\/\/<!\[CDATA\[([\s\S]*)\/\/\]\]>(\s)*$/;
const HTML_COMMENT_POSTFIX_REG_EX = /(\/\/[^\n]*|\n\s*)-->[^\n]*([\n\s]*)?$/;
Expand All @@ -16,17 +17,6 @@ const HTML_COMMENT_SIMPLE_POSTFIX_REG_EX = /-->\s*$/;
const JAVASCRIPT_PROTOCOL_REG_EX = /^\s*javascript\s*:/i;
const EXECUTABLE_SCRIPT_TYPES_REG_EX = /^\s*(application\/(x-)?(ecma|java)script|text\/(javascript(1\.[0-5])?|((x-)?ecma|x-java|js|live)script))\s*$/;

const URL_ATTR_TAGS = {
href: ['a', 'link', 'image', 'area', 'base'],
src: ['img', 'embed', 'script', 'source', 'video', 'audio', 'input', 'frame', 'iframe'],
action: ['form'],
formaction: ['button', 'input'],
manifest: ['html'],
data: ['object']
};

const URL_ATTRS = Object.keys(URL_ATTR_TAGS);

const SVG_XLINK_HREF_TAGS = [
'animate', 'animateColor', 'animateMotion', 'animateTransform', 'mpath', 'set', //animation elements
'linearGradient', 'radialGradient', 'stop', //gradient elements
Expand Down Expand Up @@ -55,7 +45,6 @@ export default class DomProcessor {
this.adapter = adapter;
this.adapter.attachEventEmitter(this);

this.URL_ATTRS = URL_ATTRS;
this.SVG_XLINK_HREF_TAGS = SVG_XLINK_HREF_TAGS;
this.AUTOCOMPLETE_ATTRIBUTE_ABSENCE_MARKER = AUTOCOMPLETE_ATTRIBUTE_ABSENCE_MARKER;

Expand Down Expand Up @@ -98,17 +87,6 @@ export default class DomProcessor {
return JAVASCRIPT_PROTOCOL_REG_EX.test(value);
}

static getAttrsForCleaning () {
const attrs = [];

for (const attr of URL_ATTRS)
attrs.push({ attr, storedAttr: DomProcessor.getStoredAttrName(attr) });

attrs.push({ attr: 'autocomplete', storedAttr: DomProcessor.getStoredAttrName('autocomplete') });

return attrs;
}

static _isHtmlImportLink (tagName, relAttr) {
return tagName && relAttr && tagName === 'link' && relAttr === 'import';
}
Expand Down
10 changes: 10 additions & 0 deletions test/client/fixtures/utils/html-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ test('shadow ui elements', function () {
strictEqual(htmlUtils.cleanUpHtml(html), '<head></head><body></body>');
});

test('attribute with a special proxying logic (GH-1448)', function () {
var div = document.createElement('div');

div.setAttribute('style', 'color: black;');

var expectedOuterHtml = '<div style="color: black;"></div>';

strictEqual(getProperty(div, 'outerHTML'), expectedOuterHtml);
});

test('form', function () {
var storedGetProxyUrl = urlUtils.getProxyUrl;

Expand Down

0 comments on commit 293d5fa

Please sign in to comment.