Skip to content

Commit

Permalink
fix: added srcset proxying (#2832)
Browse files Browse the repository at this point in the history
* fix: fixed function transformContentDispositionHeader

* fix: added srcset proxying

* fix: added tag `source` for parsing attribute `srcset`

* test: added client tests for srcset

* test: added server tests for srcset

* fix: fixed url replacing

* fix: removed unnecessary code

* fix: added definitions to getProxyUrl opts argument

* fix: fixed proxyless bug

* test: fixed expected https page

* fix: fixed IE errors
  • Loading branch information
Aleksey28 authored Jan 16, 2023
1 parent 5145dc0 commit 737a476
Show file tree
Hide file tree
Showing 16 changed files with 143 additions and 22 deletions.
10 changes: 10 additions & 0 deletions src/client/sandbox/native-methods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ class NativeMethods {
inputRequiredSetter: any;
textAreaValueSetter: any;
imageSrcSetter: any;
imageSrcsetSetter: any;
scriptSrcSetter: any;
embedSrcSetter: any;
sourceSrcSetter: any;
Expand Down Expand Up @@ -267,6 +268,7 @@ class NativeMethods {
inputRequiredGetter: any;
textAreaValueGetter: any;
imageSrcGetter: any;
imageSrcsetGetter: any;
scriptSrcGetter: any;
embedSrcGetter: any;
sourceSrcGetter: any;
Expand Down Expand Up @@ -722,6 +724,7 @@ class NativeMethods {
const inputRequiredDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLInputElement.prototype, 'required');
const textAreaValueDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLTextAreaElement.prototype, 'value');
const imageSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLImageElement.prototype, 'src');
const imageSrcsetDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLImageElement.prototype, 'srcset');
const scriptSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLScriptElement.prototype, 'src');
const scriptIntegrityDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLScriptElement.prototype, 'integrity');
const embedSrcDescriptor = win.Object.getOwnPropertyDescriptor(win.HTMLEmbedElement.prototype, 'src');
Expand Down Expand Up @@ -964,6 +967,13 @@ class NativeMethods {
this.htmlManifestSetter = htmlManifestDescriptor.set;
}

// NOTE: IE11 doesn't support the 'srcset' property
if (iframeSrcdocDescriptor) {
this.imageSrcsetSetter = imageSrcsetDescriptor.set;
this.imageSrcsetGetter = imageSrcsetDescriptor.get;
}


this.titleElementTextGetter = titleElementTextDescriptor.get;

// MutationRecord
Expand Down
16 changes: 9 additions & 7 deletions src/client/sandbox/node/element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { ATTRS_WITH_SPECIAL_PROXYING_LOGIC } from '../../../processing/dom/attri
import settings from '../../settings';
import { overrideDescriptor, overrideFunction } from '../../utils/overriding';
import InsertPosition from '../../utils/insert-position';
import { isFirefox } from '../../utils/browser';
import { isFirefox, isIE } from '../../utils/browser';
import UploadSandbox from '../upload';
import IframeSandbox from '../iframe';
import EventSandbox from '../event';
Expand Down Expand Up @@ -86,13 +86,17 @@ export default class ElementSandbox extends SandboxBase {
return;

const imgSrc = nativeMethods.imageSrcGetter.call(img);
const imgSrcset = isIE ? null : nativeMethods.imageSrcsetGetter.call(img); // NOTE: IE11 doesn't support the 'srcset' property
const skipNextLoadEvent = !!imgSrc && img.complete && !img[INTERNAL_PROPS.cachedImage];

img[INTERNAL_PROPS.forceProxySrcForImage] = true;

if (imgSrc)
img.setAttribute('src', imgSrc);

if (imgSrcset)
img.setAttribute('srcset', imgSrcset);

img[INTERNAL_PROPS.skipNextLoadEventForImage] = skipNextLoadEvent;
}

Expand Down Expand Up @@ -148,6 +152,7 @@ export default class ElementSandbox extends SandboxBase {
const tagName = domUtils.getTagName(el);
const isUrlAttr = domProcessor.isUrlAttr(el, attr, ns);
const isEventAttr = domProcessor.EVENTS.indexOf(attr) !== -1;
const isUrlsSet = attr === 'srcset';

let needToCallTargetChanged = false;
let needToRecalcHref = false;
Expand Down Expand Up @@ -201,15 +206,12 @@ export default class ElementSandbox extends SandboxBase {
else {
args[valueIndex] = isIframe && isCrossDomainUrl
? urlUtils.getCrossDomainIframeProxyUrl(value)
: urlUtils.getProxyUrl(value, { resourceType, charset: elCharset, doc: currentDocument });
: urlUtils.getProxyUrl(value, { resourceType, charset: elCharset, doc: currentDocument, isUrlsSet });
}
}
}
else if (value && !isSpecialPage && !urlUtils.parseProxyUrl(value)) {
args[valueIndex] = el[INTERNAL_PROPS.forceProxySrcForImage]
? urlUtils.getProxyUrl(value)
: urlUtils.resolveUrlAsDest(value);
}
else if (value && !isSpecialPage && !urlUtils.parseProxyUrl(value))
args[valueIndex] = urlUtils.resolveUrlAsDest(value, isUrlsSet);

if (!nativeMethods.nodeParentNodeGetter.call(el)) {
nativeMethods.objectDefineProperty(el, INTERNAL_PROPS.currentBaseUrl, {
Expand Down
2 changes: 1 addition & 1 deletion src/client/sandbox/node/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ export default class WindowSandbox extends SandboxBase {
else if (!isValidUrl(attrValue))
return urlResolver.resolve(attrValue, currentDocument);

return resolveUrlAsDest(attrValue);
return resolveUrlAsDest(attrValue, attr === 'srcset');
}

static _removeProcessingInstructions (text: string): string {
Expand Down
2 changes: 1 addition & 1 deletion src/client/sandbox/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export default class XhrSandbox extends SandboxBaseWithDelayedSettings {
overrideFunction(xmlHttpRequestProto, 'open', function (this: XMLHttpRequest, ...args: Parameters<XMLHttpRequest['open']>) { // eslint-disable-line consistent-return
let url = args[1];

if (getProxyUrl(url, xhrSandbox.proxyless) === url) {
if (getProxyUrl(url, {}, xhrSandbox.proxyless) === url) {
XhrSandbox.setRequestOptions(this, this.withCredentials, args);

return void nativeMethods.xhrOpen.apply(this, args);
Expand Down
13 changes: 9 additions & 4 deletions src/client/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as sharedUrlUtils from '../../utils/url';
import * as destLocation from './destination-location';
import urlResolver from './url-resolver';
import settings from '../settings';
import { ResourceType } from '../../typings/url';
import { ResourceType, ProxyUrlOptions } from '../../typings/url';
import globalContextInfo from './global-context-info';


Expand Down Expand Up @@ -45,7 +45,12 @@ function getCharsetFromDocument (parsedResourceType: ResourceType): string | nul
return self.document && document[INTERNAL_PROPS.documentCharset] || null;
}

export let getProxyUrl = function (url: string | URL, opts?, proxyless = false): string {
export let getProxyUrl = function (url: string | URL, opts: Partial<ProxyUrlOptions> = {}, proxyless = false): string {
if (opts.isUrlsSet) {
opts.isUrlsSet = false;
return sharedUrlUtils.handleUrlsSet(getProxyUrl, String(url), opts, proxyless);
}

if (proxyless)
return String(url);

Expand Down Expand Up @@ -232,8 +237,8 @@ export function getCrossDomainProxyPort (proxyPort: string) {
: settings.get().crossDomainProxyPort;
}

export let resolveUrlAsDest = function (url: string) {
return sharedUrlUtils.resolveUrlAsDest(url, getProxyUrl);
export let resolveUrlAsDest = function (url: string, isUrlsSet = false) {
return sharedUrlUtils.resolveUrlAsDest(url, getProxyUrl, isUrlsSet);
};

export function overrideResolveUrlAsDest (func: typeof resolveUrlAsDest): void {
Expand Down
1 change: 1 addition & 0 deletions src/processing/dom/attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
export const URL_ATTR_TAGS = {
href: ['a', 'link', 'image', 'area', 'base'],
src: ['img', 'embed', 'script', 'source', 'video', 'audio', 'input', 'frame', 'iframe'],
srcset: ['img', 'source'],
action: ['form'],
formaction: ['button', 'input'],
manifest: ['html'],
Expand Down
15 changes: 12 additions & 3 deletions src/processing/dom/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ interface ElementProcessingPattern {
relAttr?: string;
}

type UrlReplacer = (url: string, resourceType?: string, charset?: string, isCrossDomain?: boolean) => string;
type UrlReplacer = (url: string, resourceType?: string, charset?: string, isCrossDomain?: boolean, isUrlsSet?: boolean) => string;

export default class DomProcessor {
readonly HTML_PROCESSING_REQUIRED_EVENT = 'hammerhead|event|html-processing-required';
Expand Down Expand Up @@ -151,6 +151,8 @@ export default class DomProcessor {

HAS_SRC_ATTR: (el: HTMLElement) => this.isUrlAttr(el, 'src'),

HAS_SRCSET_ATTR: (el: HTMLElement) => this.isUrlAttr(el, 'srcset'),

HAS_ACTION_ATTR: (el: HTMLElement) => this.isUrlAttr(el, 'action'),

HAS_FORMACTION_ATTR: (el: HTMLElement) => this.isUrlAttr(el, 'formaction'),
Expand Down Expand Up @@ -226,6 +228,12 @@ export default class DomProcessor {
targetAttr: 'target',
elementProcessors: [this._processTargetBlank, this._processUrlAttrs, this._processUrlJsAttr],
},
{
selector: selectors.HAS_SRCSET_ATTR,
urlAttr: 'srcset',
targetAttr: 'target',
elementProcessors: [this._processTargetBlank, this._processUrlAttrs, this._processUrlJsAttr],
},
{
selector: selectors.HAS_ACTION_ATTR,
urlAttr: 'action',
Expand Down Expand Up @@ -612,6 +620,7 @@ export default class DomProcessor {
const isIframe = elTagName === 'iframe' || elTagName === 'frame';
const isScript = elTagName === 'script';
const isAnchor = elTagName === 'a';
const isUrlsSet = pattern.urlAttr === 'srcset';
const target = pattern.targetAttr ? this.adapter.getAttr(el, pattern.targetAttr) : null;

// NOTE: Elements with target=_parent shouldn’t be processed on the server,because we don't
Expand All @@ -635,9 +644,9 @@ export default class DomProcessor {
isCrossDomainSrc = !this.adapter.sameOriginCheck(parsedProxyUrl.destUrl, resourceUrl);

if ((!isSpecialPageUrl || isAnchor) && !isImgWithoutSrc && !isIframeWithEmptySrc) {
proxyUrl = elTagName === 'img' && !this.forceProxySrcForImage
proxyUrl = elTagName === 'img' && !this.forceProxySrcForImage && !isUrlsSet
? resolveUrlAsDest(resourceUrl, urlReplacer)
: urlReplacer(resourceUrl, resourceType, charsetAttrValue, isCrossDomainSrc);
: urlReplacer(resourceUrl, resourceType, charsetAttrValue, isCrossDomainSrc, isUrlsSet);
}

this.adapter.setAttr(el, storedUrlAttr, resourceUrl);
Expand Down
5 changes: 4 additions & 1 deletion src/processing/resources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ const DISK_RE = /^[A-Za-z]:/;
const RESOURCE_PROCESSORS = [pageProcessor, manifestProcessor, scriptProcessor, stylesheetProcessor];

function getResourceUrlReplacer (ctx: RequestPipelineContext): Function {
return function (resourceUrl: string, resourceType: string, charsetAttrValue: string, baseUrl: string, isCrossDomain: boolean) {
return function urlReplacer (resourceUrl: string, resourceType: string, charsetAttrValue: string, baseUrl: string, isCrossDomain = false, isUrlsSet = false) {
if (isUrlsSet)
return urlUtil.handleUrlsSet(urlReplacer, resourceUrl, resourceType, charsetAttrValue, baseUrl, isCrossDomain);

if (!urlUtil.isSupportedProtocol(resourceUrl) && !urlUtil.isSpecialPage(resourceUrl))
return resourceUrl;

Expand Down
4 changes: 2 additions & 2 deletions src/processing/resources/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ class PageProcessor extends ResourceProcessorBase {
return this.RESTART_PROCESSING;

const domProcessor = new DomProcessor(domAdapter);
const replacer = (resourceUrl, resourceType, charsetAttrValue, isCrossDomain = false) =>
urlReplacer(resourceUrl, resourceType, charsetAttrValue, baseUrl, isCrossDomain);
const replacer = (resourceUrl, resourceType, charsetAttrValue, isCrossDomain = false, isUrlsSet = false) =>
urlReplacer(resourceUrl, resourceType, charsetAttrValue, baseUrl, isCrossDomain, isUrlsSet);

domProcessor.forceProxySrcForImage = ctx.session.requestHookEventProvider.hasRequestEventListeners();
domProcessor.allowMultipleWindows = ctx.session.options.allowMultipleWindows;
Expand Down
2 changes: 1 addition & 1 deletion src/request-pipeline/header-transforms/transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function processSetCookieHeader (src: string | string[], ctx: RequestPipelineCon
}

function transformContentDispositionHeader (src: string, ctx: RequestPipelineContext) {
return ctx.contentInfo.isAttachment && !(src && src.includes('attachment')) ? 'attachment;' + src || '' : src;
return ctx.contentInfo.isAttachment && !(src && src.includes('attachment')) ? 'attachment;' + (src || '') : src;
}

// Request headers
Expand Down
2 changes: 2 additions & 0 deletions src/typings/url.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,6 @@ export interface ProxyUrlOptions {
proxyPort: string;
windowId?: string;
credentials?: Credentials;
isUrlsSet?: boolean;
doc?: Document;
}
22 changes: 21 additions & 1 deletion src/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,10 @@ export function isSupportedProtocol (url: string): boolean {
return SUPPORTED_PROTOCOL_RE.test(protocol[0]);
}

export function resolveUrlAsDest (url: string, getProxyUrlMeth: Function): string {
export function resolveUrlAsDest (url: string, getProxyUrlMeth: Function, isUrlsSet = false): string {
if (isUrlsSet)
return handleUrlsSet(resolveUrlAsDest, url, getProxyUrlMeth);

getProxyUrlMeth = getProxyUrlMeth || getProxyUrl;

if (isSupportedProtocol(url)) {
Expand Down Expand Up @@ -363,6 +366,23 @@ export function formatUrl (parsedUrl: ParsedUrl): string {
return url;
}

export function handleUrlsSet (handler: Function, url: string, ...args) {
const resourceUrls = url.split(',');
const replacedUrls = [] as string[];

for (const fullUrlStr of resourceUrls) {
const [urlStr, postUrlStr] = fullUrlStr.replace(/ +/g, ' ').trim().split(' ');

if (urlStr) {
const replacedUrl = handler(urlStr, ...args);

replacedUrls.push(replacedUrl + (postUrlStr ? ` ${postUrlStr}` : ''));
}
}

return replacedUrls.join(',');
}

export function correctMultipleSlashes (url: string, pageProtocol = ''): string {
// NOTE: Remove unnecessary slashes from the beginning of the url and after scheme.
// For example:
Expand Down
62 changes: 61 additions & 1 deletion test/client/fixtures/sandbox/node/dom-processor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var urlUtils = hammerhead.utils.url;
var sharedUrlUtils = hammerhead.sharedUtils.url;
var destLocation = hammerhead.utils.destLocation;
var eventSimulator = hammerhead.sandbox.event.eventSimulator;
var browserUtils = hammerhead.utils.browser;

var nativeMethods = hammerhead.nativeMethods;
var elementSandbox = hammerhead.sandbox.node.element;
Expand Down Expand Up @@ -634,7 +635,16 @@ test('add element with `formaction` tag to the form', function () {
});


module('should create a proxy url for the img src attribute if the image has the load handler (GH-651)', function () {
module('should create a proxy url for the img src and srcset attributes if the image has the load handler (GH-651)', function () {
function createUrlsSet (url, size) {
var urlsSet = [];

for (var i = 0; i < size; i++)
urlsSet.push(url + ' ' + (i + 1) + 'x');

return urlsSet.join(',');
}

module('onload property', function () {
var origin = location.origin || location.protocol + location.host;

Expand Down Expand Up @@ -798,6 +808,56 @@ module('should create a proxy url for the img src attribute if the image has the

strictEqual(nativeMethods.imageSrcGetter.call(img), imgProxyUrl);
});

// NOTE: IE11 doesn't support the 'srcset' property
if (!browserUtils.isIE) {
test('attach the load handler before setting up the srcset', function () {
var img = document.createElement('img');
var imgUrl = window.QUnitGlobals.getResourceUrl('../../../data/node-sandbox/image.png');
var imgProxyUrl = urlUtils.getProxyUrl(imgUrl);
var listener = function () {};
var setSize = 2;

var imgUrlsSet = createUrlsSet(imgUrl, setSize);
var imgProxyUrlsSet = createUrlsSet(imgProxyUrl, setSize);

img.addEventListener('load', listener);
img.setAttribute('srcset', imgUrlsSet);

strictEqual(nativeMethods.imageSrcsetGetter.call(img), imgProxyUrlsSet);

img.removeEventListener('load', listener);

strictEqual(nativeMethods.imageSrcsetGetter.call(img), imgProxyUrlsSet);

img.setAttribute('src', imgUrlsSet);

strictEqual(nativeMethods.imageSrcsetGetter.call(img), imgProxyUrlsSet);
});

test('attach the load handler after setting up the srcset', function () {
var img = document.createElement('img');
var imgUrl = window.QUnitGlobals.getResourceUrl('../../../data/node-sandbox/image.png');
var imgProxyUrl = urlUtils.getProxyUrl(imgUrl);
var setSize = 2;

var imgUrlsSet = createUrlsSet(imgUrl, setSize);
var imgProxyUrlsSet = createUrlsSet(imgProxyUrl, setSize);

img.setAttribute('srcset', imgUrlsSet);

var imgSrcset = nativeMethods.imageSrcsetGetter.call(img).split(',');

for (let i = 0; i < imgSrcset.length; i++)
imgSrcset[i] = urlUtils.parseUrl(imgSrcset[i]).partAfterHost;

strictEqual(imgSrcset.join(','), imgUrlsSet);

img.addEventListener('load', function () {});

strictEqual(nativeMethods.imageSrcsetGetter.call(img), imgProxyUrlsSet);
});
}
});
});

Expand Down
3 changes: 3 additions & 0 deletions test/server/data/page/expected-https.html
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,7 @@
<button formaction="https://127.0.0.1:1836/sessionId*12345!f/http://button.formaction.com/" formaction-hammerhead-stored-value="http://button.formaction.com/"></button>
<iframe srcdoc="<html><head><meta class=&quot;charset-hammerhead-shadow-ui&quot; charset=&quot;utf-8&quot;><script class=&quot;self-removing-script-hammerhead-shadow-ui&quot;>(function () {var currentScript = document.currentScript;if (!currentScript) {var scripts = document.scripts;var scriptsLength = scripts.length;currentScript = scripts[scriptsLength - 1];}currentScript.parentNode.removeChild(currentScript);var parentHammerhead = null;if (!window[&quot;%hammerhead%&quot;])Object.defineProperty(window, &quot;hammerhead|document-was-cleaned&quot;, { value: true, configurable: true });try {parentHammerhead = window.parent[&quot;%hammerhead%&quot;];} catch(e) {}if (parentHammerhead)parentHammerhead.sandbox.onIframeDocumentRecreated(window.frameElement);})();</script></head><body><a href=&quot;https://127.0.0.1:1836/sessionId*12345!i/http://link.url&quot; href-hammerhead-stored-value=&quot;http://link.url&quot;>link</a></body></html>" srcdoc-hammerhead-stored-value="<a href='http://link.url'>link</a>"></iframe>
<iframe src="https://127.0.0.1:1837/sessionId*12345!i!127.0.0.1%3A2000/http://cross.domain.com" src-hammerhead-stored-value="http://cross.domain.com"></iframe>
<img srcset="https://127.0.0.1:1836/sessionId*12345/http://domain.com/test1.jpeg 1x,https://127.0.0.1:1836/sessionId*12345/http://domain.com/test2.jpeg 2x" srcset-hammerhead-stored-value="http://domain.com/test1.jpeg 1x, http://domain.com/test2.jpeg 2x">
<img srcset="https://127.0.0.1:1836/sessionId*12345/http://domain.com/test1.jpeg 1x,https://127.0.0.1:1836/sessionId*12345/http://domain.com/test2.jpeg 2x" srcset-hammerhead-stored-value="http://domain.com/test1.jpeg 1x, http://domain.com/test2.jpeg 2x,">
<img srcset="https://127.0.0.1:1836/sessionId*12345/http://domain.com/test1.jpeg 480w,https://127.0.0.1:1836/sessionId*12345/http://domain.com/test2.jpeg 2x" srcset-hammerhead-stored-value="http://domain.com/test1.jpeg 480w, http://domain.com/test2.jpeg 2x,">
</body></html>
Loading

0 comments on commit 737a476

Please sign in to comment.