From 8a35ccc4feeae4007859c8a93afe2eb015ece87a Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 30 Apr 2020 14:07:36 -0700 Subject: [PATCH 1/5] Patching lodash's template --- src/setup_node_env/harden.js | 4 +++ src/setup_node_env/patches/lodash.js | 39 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 src/setup_node_env/patches/lodash.js diff --git a/src/setup_node_env/harden.js b/src/setup_node_env/harden.js index 466cbfa0d92cf..ed2c297f8d607 100644 --- a/src/setup_node_env/harden.js +++ b/src/setup_node_env/harden.js @@ -22,3 +22,7 @@ var hook = require('require-in-the-middle'); hook(['child_process'], function(exports, name) { return require(`./patches/${name}`)(exports); // eslint-disable-line import/no-dynamic-require }); + +hook(['lodash'], function(exports, name) { + return require(`./patches/${name}`)(exports); // eslint-disable-line import/no-dynamic-require +}); diff --git a/src/setup_node_env/patches/lodash.js b/src/setup_node_env/patches/lodash.js new file mode 100644 index 0000000000000..9390f12cc320f --- /dev/null +++ b/src/setup_node_env/patches/lodash.js @@ -0,0 +1,39 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +module.exports = function(lodash) { + lodash.template = new Proxy(lodash.template, { + apply: function(target, thisArg, args) { + var options; + if (args.length === 1) { + options = { + sourceURL: '', + }; + } else { + options = { ...args[1] }; + options.sourceURL = (options.sourceURL + '').replace(/\s/g, ' '); + } + + args[1] = options; + return target.apply(thisArg, args); + }, + }); + + return lodash; +}; From c7ed319a2746cef73a9580a71e2746957e62aeb1 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 30 Apr 2020 14:37:04 -0700 Subject: [PATCH 2/5] And now some tests! --- test/harden/lodash.js | 94 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 test/harden/lodash.js diff --git a/test/harden/lodash.js b/test/harden/lodash.js new file mode 100644 index 0000000000000..c0f439810a6a1 --- /dev/null +++ b/test/harden/lodash.js @@ -0,0 +1,94 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +require('../../src/setup_node_env'); +const _ = require('elasticsearch/node_modules/lodash'); // our fork has been patched already, using a non-patched version +const test = require('tape'); + +Object.prototype.sourceURL = '\u2028\u2029\n;global.whoops=true'; // eslint-disable-line no-extend-native + +test.onFinish(() => { + delete Object.prototype.sourceURL; +}); + +test('test setup ok', t => { + t.equal({}.sourceURL, '\u2028\u2029\n;global.whoops=true'); + t.end(); +}); + +test(`_.template('<%= foo %>')`, t => { + const output = _.template('<%= foo %>')({ foo: 'bar' }); + t.equal(output, 'bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +test(`_.template('<%= foo %>', {})`, t => { + const output = _.template('<%= foo %>', Object.freeze({}))({ foo: 'bar' }); + t.equal(output, 'bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +test(`_.template('<%= data.foo %>', { variable: 'data' })`, t => { + const output = _.template('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' }); + t.equal(output, 'bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +test(`_.template('<%= foo %>', { sourceURL: '/foo/bar' })`, t => { + // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do + const template = _.template('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' })); + t.plan(2); + try { + template(); + } catch (err) { + const path = parsePathFromStack(err.stack); + t.equal(path, '/foo/bar'); + t.equal(global.whoops, undefined); + } +}); + +test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, t => { + // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do + const template = _.template( + '<% throw new Error() %>', + Object.freeze({ sourceURL: '\u2028\u2029\nglobal.whoops=true' }) + ); + t.plan(2); + try { + template(); + } catch (err) { + console.log(err.stack); + const path = parsePathFromStack(err.stack); + t.equal(path, 'global.whoops=true'); + t.equal(global.whoops, undefined); + } +}); + +function parsePathFromStack(stack) { + const lines = stack.split('\n'); + // the frame starts at the second line + const frame = lines[1]; + + // the path is in parathensis, and ends with a colon before the line/column numbers + const [, path] = /\(([^:]+)/.exec(frame); + return path; +} From 5b44865d266b27ff5808caa09c4aac63bde9abe3 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 30 Apr 2020 18:16:56 -0700 Subject: [PATCH 3/5] Removing errant console.log --- test/harden/lodash.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/harden/lodash.js b/test/harden/lodash.js index c0f439810a6a1..c0d2988e0839f 100644 --- a/test/harden/lodash.js +++ b/test/harden/lodash.js @@ -76,7 +76,6 @@ test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true try { template(); } catch (err) { - console.log(err.stack); const path = parsePathFromStack(err.stack); t.equal(path, 'global.whoops=true'); t.equal(global.whoops, undefined); From a0bc325b813493495c7436b90d477063753689c2 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Wed, 1 Jul 2020 18:06:23 +0100 Subject: [PATCH 4/5] chore(NA): extend patches to lodash/fp and test for using template as an iteratee --- src/setup_node_env/harden.js | 4 ++-- src/setup_node_env/patches/lodash.js | 6 +++--- test/harden/lodash.js | 30 ++++++++++++++++++++++------ 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/setup_node_env/harden.js b/src/setup_node_env/harden.js index c74cde5d5d80a..90bae0722eb31 100644 --- a/src/setup_node_env/harden.js +++ b/src/setup_node_env/harden.js @@ -23,6 +23,6 @@ hook(['child_process'], function (exports, name) { return require(`./patches/${name}`)(exports); // eslint-disable-line import/no-dynamic-require }); -hook(['lodash'], function(exports, name) { - return require(`./patches/${name}`)(exports); // eslint-disable-line import/no-dynamic-require +hook(['lodash', 'lodash/fp'], function (exports) { + return require(`./patches/lodash`)(exports); // eslint-disable-line import/no-dynamic-require }); diff --git a/src/setup_node_env/patches/lodash.js b/src/setup_node_env/patches/lodash.js index 9390f12cc320f..206ebc033553b 100644 --- a/src/setup_node_env/patches/lodash.js +++ b/src/setup_node_env/patches/lodash.js @@ -17,16 +17,16 @@ * under the License. */ -module.exports = function(lodash) { +module.exports = function (lodash) { lodash.template = new Proxy(lodash.template, { - apply: function(target, thisArg, args) { + apply: function (target, thisArg, args) { var options; if (args.length === 1) { options = { sourceURL: '', }; } else { - options = { ...args[1] }; + options = Object.assign({}, args[1]); options.sourceURL = (options.sourceURL + '').replace(/\s/g, ' '); } diff --git a/test/harden/lodash.js b/test/harden/lodash.js index c0d2988e0839f..331bacb876b9f 100644 --- a/test/harden/lodash.js +++ b/test/harden/lodash.js @@ -19,6 +19,7 @@ require('../../src/setup_node_env'); const _ = require('elasticsearch/node_modules/lodash'); // our fork has been patched already, using a non-patched version +const _fp = require('elasticsearch/node_modules/lodash'); // our fork has been patched already, using a non-patched version const test = require('tape'); Object.prototype.sourceURL = '\u2028\u2029\n;global.whoops=true'; // eslint-disable-line no-extend-native @@ -27,33 +28,33 @@ test.onFinish(() => { delete Object.prototype.sourceURL; }); -test('test setup ok', t => { +test('test setup ok', (t) => { t.equal({}.sourceURL, '\u2028\u2029\n;global.whoops=true'); t.end(); }); -test(`_.template('<%= foo %>')`, t => { +test(`_.template('<%= foo %>')`, (t) => { const output = _.template('<%= foo %>')({ foo: 'bar' }); t.equal(output, 'bar'); t.equal(global.whoops, undefined); t.end(); }); -test(`_.template('<%= foo %>', {})`, t => { +test(`_.template('<%= foo %>', {})`, (t) => { const output = _.template('<%= foo %>', Object.freeze({}))({ foo: 'bar' }); t.equal(output, 'bar'); t.equal(global.whoops, undefined); t.end(); }); -test(`_.template('<%= data.foo %>', { variable: 'data' })`, t => { +test(`_.template('<%= data.foo %>', { variable: 'data' })`, (t) => { const output = _.template('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' }); t.equal(output, 'bar'); t.equal(global.whoops, undefined); t.end(); }); -test(`_.template('<%= foo %>', { sourceURL: '/foo/bar' })`, t => { +test(`_.template('<%= foo %>', { sourceURL: '/foo/bar' })`, (t) => { // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do const template = _.template('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' })); t.plan(2); @@ -66,7 +67,7 @@ test(`_.template('<%= foo %>', { sourceURL: '/foo/bar' })`, t => { } }); -test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, t => { +test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, (t) => { // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do const template = _.template( '<% throw new Error() %>', @@ -82,6 +83,23 @@ test(`_.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true } }); +test(`_.template used as an iteratee call(`, (t) => { + const templateStrArr = ['<%= data.foo %>', 'example <%= data.foo %>']; + const output = _.map(templateStrArr, _.template); + + t.equal(output[0]({ data: { foo: 'bar' } }), 'bar'); + t.equal(output[1]({ data: { foo: 'bar' } }), 'example bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +test(`_fp.template('<%= foo %>')`, (t) => { + const output = _fp.template('<%= foo %>')({ foo: 'bar' }); + t.equal(output, 'bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + function parsePathFromStack(stack) { const lines = stack.split('\n'); // the frame starts at the second line From 5b3ba3e8a8d590c9ffddd58cd11c3e168a4ae1e1 Mon Sep 17 00:00:00 2001 From: kobelb Date: Thu, 2 Jul 2020 10:08:23 -0700 Subject: [PATCH 5/5] Separate patch for 'lodash/fp' --- package.json | 1 + src/setup_node_env/harden.js | 6 +- src/setup_node_env/patches/lodash_fp.js | 33 +++++++ test/harden/lodash.js | 10 +-- test/harden/lodash_fp.js | 114 ++++++++++++++++++++++++ test/harden/package.json | 8 ++ 6 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 src/setup_node_env/patches/lodash_fp.js create mode 100644 test/harden/lodash_fp.js create mode 100644 test/harden/package.json diff --git a/package.json b/package.json index b520be4df6969..f990873469e1d 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,7 @@ "examples/*", "test/plugin_functional/plugins/*", "test/interpreter_functional/plugins/*", + "test/harden", "x-pack/test/functional_with_es_ssl/fixtures/plugins/*", "x-pack/test/plugin_api_integration/plugins/*" ], diff --git a/src/setup_node_env/harden.js b/src/setup_node_env/harden.js index 90bae0722eb31..fc9a65c0e23b3 100644 --- a/src/setup_node_env/harden.js +++ b/src/setup_node_env/harden.js @@ -23,6 +23,10 @@ hook(['child_process'], function (exports, name) { return require(`./patches/${name}`)(exports); // eslint-disable-line import/no-dynamic-require }); -hook(['lodash', 'lodash/fp'], function (exports) { +hook(['lodash/fp'], function (exports) { + return require(`./patches/lodash_fp`)(exports, require('lodash')); // eslint-disable-line import/no-dynamic-require +}); + +hook(['lodash'], function (exports) { return require(`./patches/lodash`)(exports); // eslint-disable-line import/no-dynamic-require }); diff --git a/src/setup_node_env/patches/lodash_fp.js b/src/setup_node_env/patches/lodash_fp.js new file mode 100644 index 0000000000000..937d33ecd24d0 --- /dev/null +++ b/src/setup_node_env/patches/lodash_fp.js @@ -0,0 +1,33 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +module.exports = function (fp, _) { + // per https://github.com/lodash/lodash/wiki/FP-Guide + // > Iteratee arguments are capped to avoid gotchas with variadic iteratees. + // this means that we can't specify thhe options in the second argument... in the proxy + // and just have everything work. Instead, we're going to use the non-FP _.template + // with just the first argument that is specified, and a hardcoded options with sourceURL of '' + fp.template = new Proxy(fp.template, { + apply: function (target, thisArg, args) { + return _.template(args[0], { sourceURL: '' }); + }, + }); + + return fp; +}; diff --git a/test/harden/lodash.js b/test/harden/lodash.js index 331bacb876b9f..284bd6d3e3e0f 100644 --- a/test/harden/lodash.js +++ b/test/harden/lodash.js @@ -18,8 +18,7 @@ */ require('../../src/setup_node_env'); -const _ = require('elasticsearch/node_modules/lodash'); // our fork has been patched already, using a non-patched version -const _fp = require('elasticsearch/node_modules/lodash'); // our fork has been patched already, using a non-patched version +const _ = require('lodash'); const test = require('tape'); Object.prototype.sourceURL = '\u2028\u2029\n;global.whoops=true'; // eslint-disable-line no-extend-native @@ -93,13 +92,6 @@ test(`_.template used as an iteratee call(`, (t) => { t.end(); }); -test(`_fp.template('<%= foo %>')`, (t) => { - const output = _fp.template('<%= foo %>')({ foo: 'bar' }); - t.equal(output, 'bar'); - t.equal(global.whoops, undefined); - t.end(); -}); - function parsePathFromStack(stack) { const lines = stack.split('\n'); // the frame starts at the second line diff --git a/test/harden/lodash_fp.js b/test/harden/lodash_fp.js new file mode 100644 index 0000000000000..0f707e2ba8884 --- /dev/null +++ b/test/harden/lodash_fp.js @@ -0,0 +1,114 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +require('../../src/setup_node_env'); +const fp = require('lodash/fp'); +const test = require('tape'); + +Object.prototype.sourceURL = '\u2028\u2029\n;global.whoops=true'; // eslint-disable-line no-extend-native + +test.onFinish(() => { + delete Object.prototype.sourceURL; +}); + +test('test setup ok', (t) => { + t.equal({}.sourceURL, '\u2028\u2029\n;global.whoops=true'); + t.end(); +}); + +test(`fp.template('<%= foo %>')`, (t) => { + const output = fp.template('<%= foo %>')({ foo: 'bar' }); + t.equal(output, 'bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +test(`fp.template('<%= foo %>', {})`, (t) => { + // fp.template ignores the second argument, this is negligible in this situation since options is an empty object + const output = fp.template('<%= foo %>', Object.freeze({}))({ foo: 'bar' }); + t.equal(output, 'bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +test(`fp.template('<%= data.foo %>', { variable: 'data' })`, (t) => { + // fp.template ignores the second argument, this causes an error to be thrown + t.plan(2); + try { + fp.template('<%= data.foo %>', Object.freeze({ variable: 'data' }))({ foo: 'bar' }); + } catch (err) { + t.equal(err.message, 'data is not defined'); + t.equal(global.whoops, undefined); + } +}); + +test(`fp.template('<%= foo %>', { sourceURL: '/foo/bar' })`, (t) => { + // fp.template ignores the second argument, the sourceURL is ignored + // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do + // our patching to hard-code the sourceURL and use non-FP _.template does slightly alter the stack-traces but it's negligible + const template = fp.template('<% throw new Error() %>', Object.freeze({ sourceURL: '/foo/bar' })); + t.plan(3); + try { + template(); + } catch (err) { + const path = parsePathFromStack(err.stack); + t.match(path, /^eval at /); + t.doesNotMatch(path, /\/foo\/bar/); + t.equal(global.whoops, undefined); + } +}); + +test(`fp.template('<%= foo %>', { sourceURL: '\\u2028\\u2029\\nglobal.whoops=true' })`, (t) => { + // fp.template ignores the second argument, the sourceURL is ignored + // throwing errors in the template and parsing the stack, which is a string, is super ugly, but all I know to do + // our patching to hard-code the sourceURL and use non-FP _.template does slightly alter the stack-traces but it's negligible + const template = fp.template( + '<% throw new Error() %>', + Object.freeze({ sourceURL: '\u2028\u2029\nglobal.whoops=true' }) + ); + t.plan(3); + try { + template(); + } catch (err) { + const path = parsePathFromStack(err.stack); + t.match(path, /^eval at /); + t.doesNotMatch(path, /\/foo\/bar/); + t.equal(global.whoops, undefined); + } +}); + +test(`fp.template used as an iteratee call(`, (t) => { + const templateStrArr = ['<%= data.foo %>', 'example <%= data.foo %>']; + const output = fp.map(fp.template)(templateStrArr); + + t.equal(output[0]({ data: { foo: 'bar' } }), 'bar'); + t.equal(output[1]({ data: { foo: 'bar' } }), 'example bar'); + t.equal(global.whoops, undefined); + t.end(); +}); + +function parsePathFromStack(stack) { + const lines = stack.split('\n'); + // the frame starts at the second line + const frame = lines[1]; + + // the path is in parathensis, and ends with a colon before the line/column numbers + const [, path] = /\(([^:]+)/.exec(frame); + return path; +} diff --git a/test/harden/package.json b/test/harden/package.json new file mode 100644 index 0000000000000..14896971b4183 --- /dev/null +++ b/test/harden/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/harden", + "version": "1.0.0", + "private": true, + "dependencies": { + "lodash": "4.17.15" + } +}