Skip to content

Commit

Permalink
fix `Images with style=background-image: url("img.png"); aren't loade…
Browse files Browse the repository at this point in the history 
…d` (close DevExpress#1212) (DevExpress#1293)

* fix `Images with style=background-image: url("img.png"); aren't loaded` (close DevExpress#1212)

* renaming

* fix tests

* fix review's issues
  • Loading branch information
@LavrovArtem @AndreyBelym
LavrovArtem authored and AndreyBelym committed Feb 28, 2019
1 parent 0ee822a commit 3da5b20
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 126 deletions.
113 changes: 30 additions & 83 deletions src/client/sandbox/code-instrumentation/properties/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,23 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
return HTML_ELEMENT_TEXT_PROPERTIES[prop];
}

static _createForStyleProperty (property) {
return {
condition: isStyle,

get: style => styleProcessor.cleanUp(style[property], urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style[property] = styleProcessor.process(value, urlUtils.getProxyUrl);
else
style[property] = value;

return value;
}
};
}

_createPropertyAccessors (window, document) {
let storedDomain = '';

Expand Down Expand Up @@ -756,89 +773,19 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
},

// Style
background: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.background, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.background = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.background;
}
},

backgroundImage: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.backgroundImage, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.backgroundImage = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.backgroundImage;
}
},

borderImage: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.borderImage, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.borderImage = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.borderImage;
}
},

cssText: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.cssText, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.cssText = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.cssText;
}
},

cursor: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.cursor, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.cursor = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.cursor;
}
},

listStyle: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.listStyle, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.listStyle = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.listStyle;
}
},

listStyleImage: {
condition: isStyle,
get: style => styleProcessor.cleanUp(style.listStyleImage, urlUtils.parseProxyUrl),

set: (style, value) => {
if (typeof value === 'string')
style.listStyleImage = styleProcessor.process(value, urlUtils.getProxyUrl);

return style.listStyleImage;
}
},
background: PropertyAccessorsInstrumentation._createForStyleProperty('background'),
backgroundImage: PropertyAccessorsInstrumentation._createForStyleProperty('backgroundImage'),
'background-image': PropertyAccessorsInstrumentation._createForStyleProperty('background-image'),
borderImage: PropertyAccessorsInstrumentation._createForStyleProperty('borderImage'),
'border-image': PropertyAccessorsInstrumentation._createForStyleProperty('border-image'),
'borderImageSource': PropertyAccessorsInstrumentation._createForStyleProperty('borderImageSource'),
'border-image-source': PropertyAccessorsInstrumentation._createForStyleProperty('border-image-source'),
listStyle: PropertyAccessorsInstrumentation._createForStyleProperty('listStyle'),
'list-style': PropertyAccessorsInstrumentation._createForStyleProperty('list-style'),
listStyleImage: PropertyAccessorsInstrumentation._createForStyleProperty('listStyleImage'),
'list-style-image': PropertyAccessorsInstrumentation._createForStyleProperty('list-style-image'),
cssText: PropertyAccessorsInstrumentation._createForStyleProperty('cssText'),
cursor: PropertyAccessorsInstrumentation._createForStyleProperty('cursor'),

styleSheets: {
condition: domUtils.isDocument,
Expand Down
19 changes: 19 additions & 0 deletions src/client/sandbox/native-methods.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,25 @@ class NativeMethods {
if (textAreaValueDescriptor && typeof textAreaValueDescriptor.set === 'function')
this.textAreaValueSetter = textAreaValueDescriptor.set;

// Stylesheets
if (win.CSSStyleDeclaration) {
this.CSSStyleDeclarationGetPropertyValue = win.CSSStyleDeclaration.prototype.getPropertyValue;
this.CSSStyleDeclarationSetProperty = win.CSSStyleDeclaration.prototype.setProperty;
this.CSSStyleDeclarationRemoveProperty = win.CSSStyleDeclaration.prototype.removeProperty;
}

if (win.MSStyleCSSProperties) {
this.MSStyleCSSPropertiesGetPropertyValue = win.MSStyleCSSProperties.prototype.getPropertyValue;
this.MSStyleCSSPropertiesSetProperty = win.MSStyleCSSProperties.prototype.setProperty;
this.MSStyleCSSPropertiesRemoveProperty = win.MSStyleCSSProperties.prototype.removeProperty;
}

if (win.CSS2Property) {
this.CSS2PropertyGetPropertyValue = win.CSS2Property.prototype.getPropertyValue;
this.CSS2PropertySetProperty = win.CSS2Property.prototype.setProperty;
this.CSS2PropertyRemoveProperty = win.CSS2Property.prototype.removeProperty;
}

this.refreshClasses(win);
}

Expand Down
56 changes: 55 additions & 1 deletion src/client/sandbox/node/window.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { processScript } from '../../../processing/script';
import styleProcessor from '../../../processing/style';
import * as destLocation from '../../utils/destination-location';
import { processHtml } from '../../utils/html';
import { isSubDomain, parseUrl, getProxyUrl, convertToProxyUrl, stringifyResourceType } from '../../utils/url';
import { isSubDomain, parseUrl, getProxyUrl, parseProxyUrl, convertToProxyUrl, stringifyResourceType } from '../../utils/url';
import { isFirefox, isIE9, isIE } from '../../utils/browser';
import { isCrossDomainWindows, isImgElement, isBlob } from '../../utils/dom';
import { isPrimitiveType } from '../../utils/types';
Expand Down Expand Up @@ -68,6 +68,39 @@ export default class WindowSandbox extends SandboxBase {
return String(reason);
}

static _wrapCSSGetPropertyValueIfNecessary (constructor, nativeGetPropertyValueFn) {
if (nativeGetPropertyValueFn) {
constructor.prototype.getPropertyValue = function (...args) {
const value = nativeGetPropertyValueFn.apply(this, args);

return styleProcessor.cleanUp(value, parseProxyUrl);
};
}
}

static _wrapCSSSetPropertyIfNecessary (constructor, nativeSetPropertyFn) {
if (nativeSetPropertyFn) {
constructor.prototype.setProperty = function (...args) {
const value = args[1];

if (typeof value === 'string')
args[1] = styleProcessor.process(value, getProxyUrl);

return nativeSetPropertyFn.apply(this, args);
};
}
}

static _wrapCSSRemovePropertyIfNecessary (constructor, nativeRemovePropertyFn) {
if (nativeRemovePropertyFn) {
constructor.prototype.removeProperty = function (...args) {
const oldValue = nativeRemovePropertyFn.apply(this, args);

return styleProcessor.cleanUp(oldValue, parseProxyUrl);
};
}
}

handleEvent (event) {
if (event.type === 'unhandledrejection' && !event.defaultPrevented) {
const reason = WindowSandbox._formatUnhandledRejectionReason(event.reason);
Expand Down Expand Up @@ -404,5 +437,26 @@ export default class WindowSandbox extends SandboxBase {
// NOTE: stab for ie9 and ie10 (GH-801)
if (window.XDomainRequest)
window.XDomainRequest = window.XMLHttpRequest;

WindowSandbox._wrapCSSGetPropertyValueIfNecessary(window.CSSStyleDeclaration,
nativeMethods.CSSStyleDeclarationGetPropertyValue);
WindowSandbox._wrapCSSGetPropertyValueIfNecessary(window.MSStyleCSSProperties,
nativeMethods.MSStyleCSSPropertiesGetPropertyValue);
WindowSandbox._wrapCSSGetPropertyValueIfNecessary(window.CSS2Property,
nativeMethods.CSS2PropertyGetPropertyValue);

WindowSandbox._wrapCSSSetPropertyIfNecessary(window.CSSStyleDeclaration,
nativeMethods.CSSStyleDeclarationSetProperty);
WindowSandbox._wrapCSSSetPropertyIfNecessary(window.MSStyleCSSProperties,
nativeMethods.MSStyleCSSPropertiesSetProperty);
WindowSandbox._wrapCSSSetPropertyIfNecessary(window.CSS2Property,
nativeMethods.CSS2PropertySetProperty);

WindowSandbox._wrapCSSRemovePropertyIfNecessary(window.CSSStyleDeclaration,
nativeMethods.CSSStyleDeclarationRemoveProperty);
WindowSandbox._wrapCSSRemovePropertyIfNecessary(window.MSStyleCSSProperties,
nativeMethods.MSStyleCSSPropertiesRemoveProperty);
WindowSandbox._wrapCSSRemovePropertyIfNecessary(window.CSS2Property,
nativeMethods.CSS2PropertyRemoveProperty);
}
}
6 changes: 6 additions & 0 deletions src/processing/script/instrumented.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ export const PROPERTIES = [
'autocomplete',
'background',
'backgroundImage',
'background-image',
'borderImage',
'border-image',
'borderImageSource',
'border-image-source',
'childElementCount',
'cookie',
'cssText',
Expand All @@ -38,7 +42,9 @@ export const PROPERTIES = [
'lastElementChild',
'length',
'listStyle',
'list-style',
'listStyleImage',
'list-style-image',
'localStorage',
'location',
'manifest',
Expand Down
66 changes: 26 additions & 40 deletions test/client/fixtures/sandbox/code-instrumentation/getters-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,47 +198,33 @@ test('CSSStyleSheet.href', function () {
urlUtils.parseProxyUrl = storedGetProxyUrl;
});

if (browserUtils.isWebKit) {
test('url in stylesheet properties', function () {
var el = document.createElement('div');
var url = 'http://some.domain.com/image.png';
var proxyUrl = urlUtils.getProxyUrl(url);
var quote = (function () {
var div = document.createElement('div');

div.style.backgroundImage = 'url(http://example.com/img.jpg)';

return div.style.backgroundImage.match(/url\((.*)http:\/\/example.com\/img.jpg/)[1];
})();
var getExpected = function (value) {
return 'url(' + quote + value + quote + ')';
};

eval(processScript('el.style.backgroundImage="url(' + url + ')"'));
strictEqual(getProperty(el.style, 'backgroundImage'), getExpected(url), 'backgroundImage');
strictEqual(el.style.backgroundImage, getExpected(proxyUrl), 'backgroundImage');

eval(processScript('el.style.background="url(' + url + ')"'));
strictEqual(getProperty(el.style, 'background'), getExpected(url), 'background');
strictEqual(el.style.background, getExpected(proxyUrl), 'background');

eval(processScript('el.style.listStyle="url(' + url + ')"'));
strictEqual(getProperty(el.style, 'listStyle'), getExpected(url), 'listStyle');
strictEqual(el.style.listStyle, getExpected(proxyUrl), 'listStyle');

eval(processScript('el.style.listStyleImage="url(' + url + ')"'));
strictEqual(getProperty(el.style, 'listStyleImage'), getExpected(url), 'listStyleImage');
strictEqual(el.style.listStyleImage, getExpected(proxyUrl), 'listStyleImage');

eval(processScript('el.style.cssText="background-image: url(' + url + ')"'));
strictEqual(getProperty(el.style, 'cssText'), 'background-image: ' + getExpected(url) + ';', 'cssText');
strictEqual(el.style.cssText, 'background-image: ' + getExpected(proxyUrl) + ';', 'cssText');

eval(processScript('el.style.cursor="url(' + url + '), auto"'));
strictEqual(getProperty(el.style, 'cursor'), getExpected(url) + ', auto', 'cursor');
strictEqual(el.style.cursor, getExpected(proxyUrl) + ', auto', 'cursor');
test('url in stylesheet properties', function () {
var el = document.createElement('div');
var url = 'http://some.domain.com/image.png';
var proxyUrl = urlUtils.getProxyUrl(url);
var cssProperties = ['background', 'backgroundImage', 'background-image', 'borderImage', 'border-image',
'borderImageSource', 'border-image-source', 'listStyle', 'list-style', 'listStyleImage',
'list-style-image', 'cssText', 'cursor'];

cssProperties.forEach(function (prop) {
var value = 'url(' + url + ')';

// NOTE: If we setup `borderImage` or `border-image` property then it affects a `borderImageSource` property.
var affectedProp = prop === 'borderImage' || prop === 'border-image' ? 'borderImageSource' : prop;

if (prop === 'cssText')
value = 'background:' + value;

el.style[prop] = value;

var nativeValue = el.style[affectedProp];
var proxiedValue = nativeValue && nativeValue.replace(url, proxyUrl);

eval(processScript('el.style["' + prop + '"]="' + value + '"'));
strictEqual(getProperty(el.style, affectedProp), nativeValue, prop);
strictEqual(el.style[affectedProp], proxiedValue, prop);
});
}
});

test('get style body', function () {
var style = document.createElement('style');
Expand Down
22 changes: 22 additions & 0 deletions test/client/fixtures/sandbox/node/window-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,3 +375,25 @@ test('the constructor field of a function should return a wrapped Function objec
Function.prototype.toString = nativeToString;
/*eslint-enable no-extend-native*/
});

test('getPropertyValue and setProperty methods of css object should be overridden (GH-1212)', function () {
var div = document.createElement('div');
var url = 'http://some.domain.com/image.png';
var proxyUrl = urlUtils.getProxyUrl(url);

div.style.setProperty('background', 'url(' + url + ')', '');

ok(div.style.background.indexOf(proxyUrl) !== -1);
ok(div.style.getPropertyValue('background').indexOf(proxyUrl) === -1);
ok(div.style.getPropertyValue('background').indexOf(url) !== -1);

var oldValue = div.style.removeProperty('background');

ok(oldValue.indexOf(proxyUrl) === -1);

div.style.setProperty('background', null);

ok(!div.style.background);
ok(!div.style.getPropertyValue('background'));
ok(!div.style.removeProperty('background'));
});
7 changes: 5 additions & 2 deletions test/server/script-processor-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,13 @@ function testProcessing (testCases) {

function testPropertyProcessing (templates) {
INSTRUMENTED_PROPERTIES.forEach(propName => {
if (propName.indexOf('-') !== -1)
return;

const testCases = templates.map(template => {
return {
src: template.src.replace(/\{0\}/g, propName),
expected: template.expected.replace(/\{0\}/g, propName)
src: template.src.replace(/\{0}/g, propName),
expected: template.expected.replace(/\{0}/g, propName)
};
});

Expand Down

0 comments on commit 3da5b20

Please sign in to comment.