Skip to content

Commit

Permalink
fix fetch API rasies TypeError when argument is URL object (close #…
Browse files Browse the repository at this point in the history 
…1613) (#1623)

* fix `fetch API rasies TypeError when argument is URL object`

* requested changes, fix Safari case

* refactoring

* requested changes

* requested changes

* requested changes: fix tests

* add URL test case

* xhr, location.href, location.assign, location.replace cases

* requested changes: fix xhr.open test, remove unnecessary testcase in window-test.js

* requested changes: refactor xhr.open test

* fix location property case

* try to fix tests

* try to fix tests

* fix location property test

* try to fix location property test

* try to fix location property test

* fix double quotation in location property test

* requested changes

* requested changes

* restore xhr.responseURL test
  • Loading branch information
@Farfurix @miherlosev
Farfurix authored and miherlosev committed Jun 6, 2018
1 parent 08620bc commit 0bb81c9
Show file tree
Hide file tree
Showing 9 changed files with 378 additions and 49 deletions.
3 changes: 3 additions & 0 deletions src/client/sandbox/code-instrumentation/location/wrapper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ export default class LocationWrapper extends EventEmitter {
return ensureTrailingSlash(href, locationUrl);
};
const getProxiedHref = href => {
if (typeof href !== 'string')
href = String(href);

href = prepareUrl(href);

if (isJsProtocol(href))
Expand Down
39 changes: 19 additions & 20 deletions src/client/sandbox/code-instrumentation/properties/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,30 +56,29 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
},

set: (owner, location) => {
if (typeof location === 'string') {
const ownerWindow = domUtils.isWindow(owner) ? owner : owner.defaultView;
const locationWrapper = LocationAccessorsInstrumentation.getLocationWrapper(ownerWindow);

if (!locationWrapper) {
if (!isJsProtocol(location)) {
const url = prepareUrl(location);
const resourceType = urlUtils.stringifyResourceType({ isIframe: true });

owner.location = destLocation.sameOriginCheck(location.toString(), url)
? urlUtils.getProxyUrl(url, { resourceType })
: urlUtils.getCrossDomainIframeProxyUrl(url);
}
else
owner.location = processJsAttrValue(location, { isJsProtocol: true, isEventAttr: false });
const ownerWindow = domUtils.isWindow(owner) ? owner : owner.defaultView;
const locationWrapper = LocationAccessorsInstrumentation.getLocationWrapper(ownerWindow);

if (!locationWrapper) {
if (typeof location !== 'string')
location = String(location);

if (!isJsProtocol(location)) {
const url = prepareUrl(location);
const resourceType = urlUtils.stringifyResourceType({ isIframe: true });

owner.location = destLocation.sameOriginCheck(location.toString(), url)
? urlUtils.getProxyUrl(url, { resourceType })
: urlUtils.getCrossDomainIframeProxyUrl(url);
}
else
// eslint-disable-next-line no-restricted-properties
locationWrapper.href = location;

return location;
owner.location = processJsAttrValue(location, { isJsProtocol: true, isEventAttr: false });
}
else
// eslint-disable-next-line no-restricted-properties
locationWrapper.href = location;

return owner.location;
return location;
}
}
};
Expand Down
32 changes: 16 additions & 16 deletions src/client/sandbox/fetch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getOriginHeader, sameOriginCheck, get as getDestLocation } from '../uti
import { isFetchHeaders, isFetchRequest } from '../utils/dom';
import SAME_ORIGIN_CHECK_FAILED_STATUS_CODE from '../../request-pipeline/xhr/same-origin-check-failed-status-code';
import { overrideDescriptor } from '../utils/property-overriding';
import * as browserUtils from '../utils/browser';

const DEFAULT_REQUEST_CREDENTIALS = nativeMethods.Request ? new nativeMethods.Request(window.location.toString()).credentials : void 0;

Expand Down Expand Up @@ -43,23 +44,16 @@ export default class FetchSandbox extends SandboxBase {
const inputIsFetchRequest = isFetchRequest(input);
let init = args[1];

if (inputIsString) {
args[0] = getProxyUrl(input);
if (!inputIsFetchRequest) {
args[0] = getProxyUrl(inputIsString ? input : String(input));
init = init || {};
args[1] = FetchSandbox._addSpecialHeadersToRequestInit(init);
}
else if (inputIsFetchRequest && init)
else if (init)
args[1] = FetchSandbox._addSpecialHeadersToRequestInit(init);
}

static _isValidRequestArgs (args) {
return typeof args[0] === 'string' || isFetchRequest(args[0]);
}

static _requestIsValid (args) {
if (!FetchSandbox._isValidRequestArgs(args))
return false;

static _sameOriginCheck (args) {
let url = null;
let requestMode = null;

Expand All @@ -68,7 +62,7 @@ export default class FetchSandbox extends SandboxBase {
requestMode = args[0].mode;
}
else {
url = args[0];
url = parseProxyUrl(args[0]).destUrl;
requestMode = (args[1] || {}).mode;
}

Expand Down Expand Up @@ -109,13 +103,19 @@ export default class FetchSandbox extends SandboxBase {
window.Request.prototype = nativeMethods.Request.prototype;

window.fetch = function (...args) {
if (!args.length)
// NOTE: Safari processed the empty `fetch()` request without `Promise` rejection (GH-1613)
if (!args.length && !browserUtils.isSafari)
return nativeMethods.fetch.apply(this);

if (!FetchSandbox._requestIsValid(args))
return sandbox.window.Promise.reject(new TypeError());
try {
FetchSandbox._processArguments(args);
}
catch (e) {
return sandbox.window.Promise.reject(e);
}

FetchSandbox._processArguments(args);
if (!FetchSandbox._sameOriginCheck(args))
return sandbox.window.Promise.reject(new TypeError());

const fetchPromise = nativeMethods.fetch.apply(this, args);

Expand Down
6 changes: 4 additions & 2 deletions src/client/sandbox/xhr.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ export default class XhrSandbox extends SandboxBase {
// NOTE: Redirect all requests to the Hammerhead proxy and ensure that requests don't
// violate Same Origin Policy.
xmlHttpRequestProto.open = function () {
if (typeof arguments[1] === 'string')
arguments[1] = getProxyUrl(arguments[1]);
const url = arguments[1];
const urlIsString = typeof input === 'string';

arguments[1] = getProxyUrl(urlIsString ? url : String(url));

nativeMethods.xhrOpen.apply(this, arguments);
};
Expand Down
14 changes: 14 additions & 0 deletions test/client/config-qunit-server-app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,18 @@ module.exports = function (app) {

delete cookies[userAgent];
});

// We should add routes for iframe loading in IE 11 ("location" property test) (GH-1613)
var iframeLocationUrlCallback = function (req, res, next) {
var locationPossibleValues = ['null', 'undefined', '[object Object]', 'some-path'];

if (locationPossibleValues.includes(req.params.url))
res.send(req.params.url);
else
next();
};

app.get('/fixtures/sandbox/code-instrumentation/:url', iframeLocationUrlCallback);

app.get('/:url', iframeLocationUrlCallback);
};
198 changes: 198 additions & 0 deletions test/client/fixtures/sandbox/code-instrumentation/location-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,204 @@ test('special pages (GH-339)', function () {
destLocation.forceLocation(storedForcedLocation);
});

test('different url types for locationWrapper methods (href, replace, assign) (GH-1613)', function () {
var testCases = [
{
url: null,
expectedUrl: 'https://example.com/null'
},
{
url: void 0,
expectedUrl: 'https://example.com/undefined'
},
{
url: { url: '/some-path' },
expectedUrl: 'https://example.com/[object%20Object]'
},
{
url: {
url: '/some-path',
toString: function () {
return this.url;
}
},
expectedUrl: 'https://example.com/some-path'
}
];

// NOTE: IE11 doesn't support 'URL()'
if (!browserUtils.isIE11) {
testCases.push({
url: new URL('https://example.com/some-path'),
expectedUrl: 'https://example.com/some-path'
});
}

var windowMock = {
location: {
href: '',

replace: function (url) {
this.href = url;
},

assign: function (url) {
this.href = url;
},

toString: function () {
return urlUtils.getProxyUrl(this.location.href);
}
}
};

windowMock.top = windowMock;

var locationWrapper = new LocationWrapper(windowMock);

testCases.map(function (testCase) {
locationWrapper.href = testCase.url;
strictEqual(windowMock.location.href, urlUtils.getProxyUrl(testCase.expectedUrl));

locationWrapper.replace(testCase.url);
strictEqual(windowMock.location.href, urlUtils.getProxyUrl(testCase.expectedUrl));

locationWrapper.assign(testCase.url);
strictEqual(windowMock.location.href, urlUtils.getProxyUrl(testCase.expectedUrl));
});
});

test('throwing errors on calling locationWrapper methods (href, replace, assign) with invalid arguments', function () {
expect(browserUtils.isIE11 ? 1 : 3);

var invalidUrlObject = {
toString: function () {
return {};
}
};

var windowMock = {
location: {
href: '',

replace: function (url) {
this.href = url;
},

assign: function (url) {
this.href = url;
},

toString: function () {
return urlUtils.getProxyUrl(this.location.href);
}
}
};

windowMock.top = windowMock;

var locationWrapper = new LocationWrapper(windowMock);

try {
locationWrapper.href = invalidUrlObject;
strictEqual(windowMock.location.href, urlUtils.getProxyUrl(''));
}
catch (e) {
ok(true, 'href');
}

if (!browserUtils.isIE11) {
try {
locationWrapper.replace(invalidUrlObject);
strictEqual(windowMock.location.href, urlUtils.getProxyUrl(''));
}
catch (e) {
ok(true, 'replace');
}

try {
locationWrapper.assign(invalidUrlObject);
strictEqual(windowMock.location.href, urlUtils.getProxyUrl(''));
}
catch (e) {
ok(true, 'assign');
}
}
});

test('different url types for "location" property (GH-1613)', function () {
var checkLocation = function (iframe) {
return new Promise(function (resolve) {
iframe.addEventListener('load', function () {
resolve(iframe.contentWindow.location.toString());
});
});
};

var checkLocationAssignment = function (nativeIframeUrl, processedIframeUrl) {
return Promise.all([createTestIframe(), createTestIframe()])
.then(function (iframes) {
var nativeIframePromise = checkLocation(iframes[0]);
var iframePromise = checkLocation(iframes[1]);

iframes[0].contentWindow.location = nativeIframeUrl;
eval(processScript('iframes[1].contentWindow.location = ' + processedIframeUrl));

return Promise.all([nativeIframePromise, iframePromise]);
})
.then(function (urls) {
var nativeIframeLocation = urls[0].slice(urls[0].lastIndexOf('/') + 1);
var processedIframeLocation = urls[1].slice(urls[1].lastIndexOf('/') + 1);

strictEqual(nativeIframeLocation, processedIframeLocation);
});
};

var cases = [
checkLocationAssignment(null, 'null'),
checkLocationAssignment(void 0, 'void 0'),
checkLocationAssignment({}, '{}'),
checkLocationAssignment({
toString: function () {
return '/some-path';
}
}, '{' +
' toString: function () {\n' +
' return "/some-path";\n' +
' }' +
'}')
];

// NOTE: IE11 doesn't support 'URL()'
if (!browserUtils.isIE11)
cases.push(checkLocationAssignment(new URL(location.origin + '/some-path'), 'new URL(location.origin + "/some-path")'));

return Promise.all(cases);
});

test('should not navigate in case of invalid "location" property assigment (GH-1613)', function () {
expect(0);

var invalidUrlObject = '{ toString: function () { return {} } }';

var checkLocation = function (iframe) {
return new Promise(function () {
iframe.addEventListener('load', function () {
ok(false, 'should not navigate');
});
});
};

createTestIframe()
.then(function (iframe) {
var iframePromise = checkLocation(iframe);

eval(processScript('iframe.contentWindow.location = ' + invalidUrlObject));

return iframePromise;
});
});

test('should ensure a trailing slash on page navigation using href setter, assign and replace methods (GH-1426)', function () {
function getExpectedProxyUrl (testCase) {
var proxiedUrl = urlUtils.getProxyUrl(testCase.url);
Expand Down
Loading

0 comments on commit 0bb81c9

Please sign in to comment.