Skip to content

Commit

Permalink
Ensure { runScripts: "outside-only" } also passes down to iframes
Browse files Browse the repository at this point in the history
In the process, eliminate all internal usage of ProcessExternalResources, leaving that purely as part of the old API.

This also fixed issues where window indexed accessors, e.g. window[0] or window.frames[0], would not work when runScripts was set to "dangerously".
  • Loading branch information
domenic committed Apr 29, 2017
1 parent 3d0f28d commit ca8dfef
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 52 deletions.
18 changes: 5 additions & 13 deletions lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,12 @@ class JSDOM {
// ugly, internal API.
const features = {
FetchExternalResources: [],
ProcessExternalResources: false,
SkipExternalResources: false
};
if (options.runScripts === "dangerously") {
features.ProcessExternalResources = ["script"];
}

if (options.resources === "usable") {
features.FetchExternalResources = ["link", "img", "frame", "iframe"];
if (options.runScripts === "dangerously") {
if (options.windowOptions.runScripts === "dangerously") {
features.FetchExternalResources.push("script");
}

Expand All @@ -66,11 +62,6 @@ class JSDOM {
const documentImpl = idlUtils.implForWrapper(this[window]._document);
applyDocumentFeatures(documentImpl, features);

if (options.runScripts === "outside-only") {
vm.createContext(this[window]);
this[window]._document._defaultView = this[window]._globalProxy = vm.runInContext("this", this[window]);
}

options.beforeParse(this[window]._globalProxy);

// TODO NEWAPI: this is still pretty hacky. It's also different than jsdom.jsdom. Does it work? Can it be better?
Expand Down Expand Up @@ -245,6 +236,7 @@ function transformOptions(options, encoding) {
parsingMode: "html",
userAgent: DEFAULT_USER_AGENT,
parseOptions: { locationInfo: false },
runScripts: undefined,
encoding,

// Defaults filled in later
Expand All @@ -254,7 +246,6 @@ function transformOptions(options, encoding) {

// Defaults
resources: undefined,
runScripts: undefined,
beforeParse() { }
};

Expand Down Expand Up @@ -308,8 +299,9 @@ function transformOptions(options, encoding) {
}

if (options.runScripts !== undefined) {
transformed.runScripts = String(options.runScripts);
if (transformed.runScripts !== "dangerously" && transformed.runScripts !== "outside-only") {
transformed.windowOptions.runScripts = String(options.runScripts);
if (transformed.windowOptions.runScripts !== "dangerously" &&
transformed.windowOptions.runScripts !== "outside-only") {
throw new RangeError(`runScripts must be undefined, "dangerously", or "outside-only"`);
}
}
Expand Down
33 changes: 19 additions & 14 deletions lib/jsdom/browser/Window.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"use strict";

const webIDLConversions = require("webidl-conversions");
const CSSStyleDeclaration = require("cssstyle").CSSStyleDeclaration;
const { CSSStyleDeclaration } = require("cssstyle");
const notImplemented = require("./not-implemented");
const VirtualConsole = require("../virtual-console");
const define = require("../utils").define;
const { define } = require("../utils");
const EventTarget = require("../living/generated/EventTarget");
const namedPropertiesWindow = require("../living/named-properties-window");
const cssom = require("cssom");
Expand All @@ -18,6 +18,7 @@ const createFileReader = require("../living/generated/FileReader").createInterfa
const Document = require("../living/generated/Document");
const Navigator = require("../living/generated/Navigator");
const reportException = require("../living/helpers/runtime-script-errors");
const { contextifyWindow } = require("./documentfeatures.js");

// NB: the require() must be after assigning `module.exports` because this require() is circular
// TODO: this above note might not even be true anymore... figure out the cycle and document it, or clean up.
Expand Down Expand Up @@ -54,16 +55,11 @@ function Window(options) {

///// PRIVATE DATA PROPERTIES

// vm initialization is defered until script processing is activated (in level1/core)
// vm initialization is defered until script processing is activated
this._globalProxy = this;

this.__timers = Object.create(null);

// Set up the window as if it's a top level window.
// If it's not, then references will be corrected by frame/iframe code.
this._parent = this._top = this._globalProxy;
this._frameElement = null;

// List options explicitly to be clear which are passed through
this._document = Document.create([], {
core: dom,
Expand Down Expand Up @@ -99,12 +95,6 @@ function Window(options) {
}];
this._currentSessionHistoryEntryIndex = 0;


// This implements window.frames.length, since window.frames returns a
// self reference to the window object. This value is incremented in the
// HTMLFrameElement init function (see: level2/html.js).
this._length = 0;

// TODO NEWAPI can remove this
if (options.virtualConsole) {
if (options.virtualConsole instanceof VirtualConsole) {
Expand All @@ -117,6 +107,21 @@ function Window(options) {
this._virtualConsole = new VirtualConsole();
}

this._runScripts = options.runScripts;
if (this._runScripts === "outside-only" || this._runScripts === "dangerously") {
contextifyWindow(this);
}

// Set up the window as if it's a top level window.
// If it's not, then references will be corrected by frame/iframe code.
this._parent = this._top = this._globalProxy;
this._frameElement = null;

// This implements window.frames.length, since window.frames returns a
// self reference to the window object. This value is incremented in the
// HTMLFrameElement implementation.
this._length = 0;

///// GETTERS

const navigator = Navigator.create([], { userAgent: options.userAgent });
Expand Down
14 changes: 12 additions & 2 deletions lib/jsdom/browser/documentfeatures.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
"use strict";
const vm = require("vm");
const idlUtils = require("../living/generated/utils");

exports.availableDocumentFeatures = [
"FetchExternalResources",
"ProcessExternalResources",
"SkipExternalResources"
];

exports.defaultDocumentFeatures = {
FetchExternalResources: ["script", "link"], // omitted by default: "frame"
ProcessExternalResources: ["script"],
SkipExternalResources: false
};

Expand Down Expand Up @@ -43,3 +43,13 @@ exports.applyDocumentFeatures = (documentImpl, features = {}) => {
}
}
};

exports.contextifyWindow = window => {
if (vm.isContext(window)) {
return;
}

vm.createContext(window);
const documentImpl = idlUtils.implForWrapper(window._document);
documentImpl._defaultView = window._globalProxy = vm.runInContext("this", window);
};
1 change: 0 additions & 1 deletion lib/jsdom/living/domparsing/DOMParser-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ function createScriptingDisabledDocument(parsingMode, contentType, string) {
// "scripting enabled" set to false
applyDocumentFeatures(document, {
FetchExternalResources: [],
ProcessExternalResources: false,
SkipExternalResources: false
});

Expand Down
5 changes: 3 additions & 2 deletions lib/jsdom/living/events/EventTarget-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,11 @@ function invokeInlineListeners(object, event) {
const wrapper = idlUtils.wrapperForImpl(object);
const inlineListener = getListenerForInlineEventHandler(wrapper, event.type);
if (inlineListener) {
// Will be falsy for windows that have closed
const document = object._ownerDocument || (wrapper && (wrapper._document || wrapper._ownerDocument));

// Will be falsy for windows that have closed
if (document && (!object.nodeName || document.implementation._hasFeature("ProcessExternalResources", "script"))) {
const runScripts = document && document._defaultView && document._defaultView._runScripts === "dangerously";
if (!object.nodeName || runScripts) {
invokeEventListeners([{
callback: inlineListener,
options: normalizeEventHandlerOptions(false, ["capture", "once"])
Expand Down
9 changes: 0 additions & 9 deletions lib/jsdom/living/nodes/DOMImplementation-impl.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"use strict";

const vm = require("vm");
const validateNames = require("../helpers/validate-names");
const DocumentType = require("../generated/DocumentType");
const Document = require("../generated/Document");
Expand Down Expand Up @@ -138,14 +137,6 @@ class DOMImplementationImpl {
} else {
this._features[feature].push(version);
}

if (feature === "processexternalresources" &&
(version === "script" || (version.indexOf && version.indexOf("script") !== -1)) &&
!vm.isContext(this._ownerDocument._global)) {
vm.createContext(this._ownerDocument._global);
this._ownerDocument._defaultView._globalProxy = vm.runInContext("this", this._ownerDocument._global);
this._ownerDocument._defaultView = this._ownerDocument._defaultView._globalProxy;
}
} else {
this._features[feature] = [];
}
Expand Down
3 changes: 2 additions & 1 deletion lib/jsdom/living/nodes/HTMLFrameElement-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ function loadFrame(frame) {
encoding: parentDoc._encoding,
agentOptions: parentDoc._agentOptions,
strictSSL: parentDoc._strictSSL,
proxy: parentDoc._proxy
proxy: parentDoc._proxy,
runScripts: parentDoc._defaultView._runScripts
});
const contentDoc = frame._contentDocument = idlUtils.implForWrapper(wnd._document);
applyDocumentFeatures(contentDoc, parentDoc._implementation._features);
Expand Down
2 changes: 1 addition & 1 deletion lib/jsdom/living/nodes/HTMLScriptElement-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class HTMLScriptElementImpl extends HTMLElementImpl {
_eval(text, filename) {
const typeString = this._getTypeString();

if (this._ownerDocument.implementation._hasFeature("ProcessExternalResources", "script") &&
if (this._ownerDocument._defaultView && this._ownerDocument._defaultView._runScripts === "dangerously" &&
jsMIMETypes.has(typeString.toLowerCase())) {
this._ownerDocument._writeAfterElement = this;
processJavaScript(this, text, filename);
Expand Down
42 changes: 33 additions & 9 deletions lib/old-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,22 @@ exports.jsdom = function (html, options) {
}
}

// Adapt old API `features: { ProcessExternalResources: ["script"] }` to the runScripts option.
// This is part of a larger effort to eventually remove the document features infrastructure entirely. It's unclear
// whether we'll kill the old API or document features first, but as long as old API survives, attempts to kill
// document features will need this kind of adapter.
if (!options.features) {
options.features = exports.defaultDocumentFeatures;
}
if (options.features.ProcessExternalResources === undefined) {
options.features.ProcessExternalResources = ["script"];
}
const ProcessExternalResources = options.features.ProcessExternalResources || [];
if (ProcessExternalResources === "script" ||
(ProcessExternalResources.includes && ProcessExternalResources.includes("script"))) {
options.runScripts = "dangerously";
}

// List options explicitly to be clear which are passed through
const window = new Window({
parsingMode: options.parsingMode,
Expand All @@ -127,7 +143,8 @@ exports.jsdom = function (html, options) {
agentOptions: options.agentOptions,
strictSSL: options.strictSSL,
proxy: options.proxy,
userAgent: options.userAgent
userAgent: options.userAgent,
runScripts: options.runScripts
});

const documentImpl = idlUtils.implForWrapper(window.document);
Expand Down Expand Up @@ -162,15 +179,20 @@ exports.jQueryify = exports.jsdom.jQueryify = function (window, jqueryUrl, callb
}

const implImpl = idlUtils.implForWrapper(window.document.implementation);
const features = implImpl._features;
const oldFeatures = implImpl._features;
const oldRunScripts = window._runScripts;

implImpl._addFeature("FetchExternalResources", ["script"]);
implImpl._addFeature("ProcessExternalResources", ["script"]);
documentFeatures.contextifyWindow(idlUtils.implForWrapper(window.document)._global);
window._runScripts = "dangerously";

const scriptEl = window.document.createElement("script");
scriptEl.className = "jsdom";
scriptEl.src = jqueryUrl;
scriptEl.onload = scriptEl.onerror = () => {
implImpl._features = features;
implImpl._features = oldFeatures;
window._runScripts = oldRunScripts;
// Can't un-contextify the window. Oh well. That's what we get for such magic behavior in old API.

if (callback) {
callback(window, window.jQuery);
Expand All @@ -181,7 +203,7 @@ exports.jQueryify = exports.jsdom.jQueryify = function (window, jqueryUrl, callb
};

exports.env = exports.jsdom.env = function () {
const config = getConfigFromArguments(arguments);
const config = getConfigFromEnvArguments(arguments);
let req = null;

if (config.file && canReadFilesFromFS) {
Expand Down Expand Up @@ -337,7 +359,6 @@ function processHTML(config) {

if (config.scripts.length > 0 || config.src.length > 0) {
implImpl._addFeature("FetchExternalResources", ["script"]);
implImpl._addFeature("ProcessExternalResources", ["script"]);

for (const scriptSrc of config.scripts) {
const script = window.document.createElement("script");
Expand Down Expand Up @@ -383,7 +404,7 @@ function setGlobalDefaultConfig(config) {
`Node.js (${process.platform}; U; rv:${process.version}) AppleWebKit/537.36 (KHTML, like Gecko)`;
}

function getConfigFromArguments(args) {
function getConfigFromEnvArguments(args) {
const config = {};
if (typeof args[0] === "object") {
Object.assign(config, args[0]);
Expand Down Expand Up @@ -422,8 +443,8 @@ function getConfigFromArguments(args) {

config.features = config.features || {
FetchExternalResources: false,
ProcessExternalResources: false,
SkipExternalResources: false
SkipExternalResources: false,
ProcessExternalResources: false // needed since we'll process it inside jsdom.jsdom()
};

if (!config.url && config.file) {
Expand All @@ -434,6 +455,9 @@ function getConfigFromArguments(args) {

setGlobalDefaultConfig(config);

if (config.scripts.length > 0 || config.src.length > 0) {
config.features.ProcessExternalResources = ["script"];
}
return config;
}

Expand Down
20 changes: 20 additions & 0 deletions test/api/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,26 @@ describe("API: constructor options", () => {
assert.strictEqual(dom.window.document.body.children.length, 2);
});

it("should ensure eval exists on iframes when set to \"outside-only\"", () => {
const dom = new JSDOM(`<iframe></iframe>`, { runScripts: "outside-only" });
const frameWindow = dom.window.document.querySelector("iframe").contentWindow;

frameWindow.eval(`document.body.appendChild(document.createElement("p"));`);

assert.strictEqual(frameWindow.document.body.children.length, 1);
});

it("should execute <script>s in iframes when set to \"dangerously\"", () => {
const dom = new JSDOM(`<iframe></iframe>`, { runScripts: "dangerously" });
const frameWindow = dom.window.document.querySelector("iframe").contentWindow;

frameWindow.document.open();
frameWindow.document.write(`<script>parent.prop = "i was executed";</script>`);
frameWindow.document.close();

assert.strictEqual(dom.window.prop, "i was executed");
});

it("should disallow other values", () => {
assert.throws(() => new JSDOM(``, { runScripts: null }), RangeError);
assert.throws(() => new JSDOM(``, { runScripts: "asdf" }), RangeError);
Expand Down

0 comments on commit ca8dfef

Please sign in to comment.