From e55cbb3e3dbedc5c17d0f82ed5170b156199f52d Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Thu, 3 Feb 2022 02:01:22 +0100 Subject: [PATCH 1/3] Fix bug in debug node due to msg.hasOwnProperty construct `msg.hasOwnProperty("status")` might make the debug node crash/produce an error if the payload was created with `Object.create(null)`. This is the case e.g. for `ini` (to parse INI files), an official NPM node: https://github.com/npm/ini/blob/4f289946b3bf95f144e849d771f64e4f2aa2737c/lib/ini.js#L63 My Node-RED node `node-red-contrib-parser-ini`, which is using that library, was hit by this bug and I had to ship a workaround https://github.com/alexandrainst/node-red-contrib-parser-ini/blob/fe6b1eb4b18fd54459e2505b1c2f54eb0a9c9fec/parser-ini.js#L14 The `msg.hasOwnProperty("xxx")` construct should not be used since ECMAScript 5.1. ESLint advises in the same direction https://eslint.org/docs/rules/no-prototype-builtins This patch was produced using the following regex: Search: `\b([\w.]+).hasOwnProperty\(` Replace: `Object.prototype.hasOwnProperty.call($1, ` This could be applied more gobally if desired. --- .../@node-red/nodes/core/common/21-debug.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/common/21-debug.js b/packages/node_modules/@node-red/nodes/core/common/21-debug.js index 73d364e43..c67a7d38b 100644 --- a/packages/node_modules/@node-red/nodes/core/common/21-debug.js +++ b/packages/node_modules/@node-red/nodes/core/common/21-debug.js @@ -107,7 +107,7 @@ module.exports = function(RED) { } }) this.on("input", function(msg, send, done) { - if (msg.hasOwnProperty("status") && msg.status.hasOwnProperty("source") && msg.status.source.hasOwnProperty("id") && (msg.status.source.id === node.id)) { + if (Object.prototype.hasOwnProperty.call(msg, "status") && Object.prototype.hasOwnProperty.call(msg.status, "source") && Object.prototype.hasOwnProperty.call(msg.status.source, "id") && (msg.status.source.id === node.id)) { done(); return; } @@ -118,17 +118,17 @@ module.exports = function(RED) { var st = (typeof output === 'string') ? output : util.inspect(output); var fill = "grey"; var shape = "dot"; - if (typeof output === 'object' && output.hasOwnProperty("fill") && output.hasOwnProperty("shape") && output.hasOwnProperty("text")) { + if (typeof output === 'object' && Object.prototype.hasOwnProperty.call(output, "fill") && Object.prototype.hasOwnProperty.call(output, "shape") && Object.prototype.hasOwnProperty.call(output, "text")) { fill = output.fill; shape = output.shape; st = output.text; } if (node.statusType === "auto") { - if (msg.hasOwnProperty("error")) { + if (Object.prototype.hasOwnProperty.call(msg, "error")) { fill = "red"; st = msg.error.message; } - if (msg.hasOwnProperty("status")) { + if (Object.prototype.hasOwnProperty.call(msg, "status")) { fill = msg.status.fill || "grey"; shape = msg.status.shape || "ring"; st = msg.status.text || ""; @@ -194,7 +194,7 @@ module.exports = function(RED) { function sendDebug(msg) { // don't put blank errors in sidebar (but do add to logs) - //if ((msg.msg === "") && (msg.hasOwnProperty("level")) && (msg.level === 20)) { return; } + //if ((msg.msg === "") && (Object.prototype.hasOwnProperty.call(msg, "level")) && (msg.level === 20)) { return; } msg = RED.util.encodeObject(msg,{maxLength:debuglength}); RED.comms.publish("debug",msg); } From 280d63fde7750c84e1972b3f34510c7345b75ac8 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Thu, 3 Feb 2022 15:59:25 +0100 Subject: [PATCH 2/3] Fix util.encodeObject --- .../@node-red/nodes/core/common/21-debug.js | 11 ++--- .../node_modules/@node-red/util/lib/util.js | 42 ++++++++++++------- test/nodes/core/common/21-debug_spec.js | 32 ++++++++++++++ test/unit/@node-red/util/lib/util_spec.js | 18 ++++++++ 4 files changed, 83 insertions(+), 20 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/common/21-debug.js b/packages/node_modules/@node-red/nodes/core/common/21-debug.js index c67a7d38b..92fa46967 100644 --- a/packages/node_modules/@node-red/nodes/core/common/21-debug.js +++ b/packages/node_modules/@node-red/nodes/core/common/21-debug.js @@ -7,6 +7,7 @@ module.exports = function(RED) { var debuglength = RED.settings.debugMaxLength || 1000; var useColors = RED.settings.debugUseColors || false; util.inspect.styles.boolean = "red"; + const { hasOwnProperty } = Object.prototype; function DebugNode(n) { var hasEditExpression = (n.targetType === "jsonata"); @@ -107,7 +108,7 @@ module.exports = function(RED) { } }) this.on("input", function(msg, send, done) { - if (Object.prototype.hasOwnProperty.call(msg, "status") && Object.prototype.hasOwnProperty.call(msg.status, "source") && Object.prototype.hasOwnProperty.call(msg.status.source, "id") && (msg.status.source.id === node.id)) { + if (hasOwnProperty.call(msg, "status") && hasOwnProperty.call(msg.status, "source") && hasOwnProperty.call(msg.status.source, "id") && (msg.status.source.id === node.id)) { done(); return; } @@ -118,17 +119,17 @@ module.exports = function(RED) { var st = (typeof output === 'string') ? output : util.inspect(output); var fill = "grey"; var shape = "dot"; - if (typeof output === 'object' && Object.prototype.hasOwnProperty.call(output, "fill") && Object.prototype.hasOwnProperty.call(output, "shape") && Object.prototype.hasOwnProperty.call(output, "text")) { + if (typeof output === 'object' && hasOwnProperty.call(output, "fill") && hasOwnProperty.call(output, "shape") && hasOwnProperty.call(output, "text")) { fill = output.fill; shape = output.shape; st = output.text; } if (node.statusType === "auto") { - if (Object.prototype.hasOwnProperty.call(msg, "error")) { + if (hasOwnProperty.call(msg, "error")) { fill = "red"; st = msg.error.message; } - if (Object.prototype.hasOwnProperty.call(msg, "status")) { + if (hasOwnProperty.call(msg, "status")) { fill = msg.status.fill || "grey"; shape = msg.status.shape || "ring"; st = msg.status.text || ""; @@ -194,7 +195,7 @@ module.exports = function(RED) { function sendDebug(msg) { // don't put blank errors in sidebar (but do add to logs) - //if ((msg.msg === "") && (Object.prototype.hasOwnProperty.call(msg, "level")) && (msg.level === 20)) { return; } + //if ((msg.msg === "") && (hasOwnProperty.call(msg, "level")) && (msg.level === 20)) { return; } msg = RED.util.encodeObject(msg,{maxLength:debuglength}); RED.comms.publish("debug",msg); } diff --git a/packages/node_modules/@node-red/util/lib/util.js b/packages/node_modules/@node-red/util/lib/util.js index 15f6b44d9..a81030fb3 100644 --- a/packages/node_modules/@node-red/util/lib/util.js +++ b/packages/node_modules/@node-red/util/lib/util.js @@ -24,6 +24,18 @@ const jsonata = require("jsonata"); const moment = require("moment-timezone"); const safeJSONStringify = require("json-stringify-safe"); const util = require("util"); +const { hasOwnProperty } = Object.prototype; + +/** + * Safely returns the object construtor name. + * @return {String} the name of the object constructor if it exists, empty string otherwise. + * @memberof @node-red/util_util + */ +function constructorName(obj) { + // Note: This function could be replaced by optional chaining in Node.js 14+: + // obj?.constructor?.name + return obj && obj.constructor ? obj.constructor.name : ''; +} /** * Generates a psuedo-unique-random id. @@ -171,7 +183,7 @@ function compareObjects(obj1,obj2) { } for (var k in obj1) { /* istanbul ignore else */ - if (obj1.hasOwnProperty(k)) { + if (hasOwnProperty.call(obj1, k)) { if (!compareObjects(obj1[k],obj2[k])) { return false; } @@ -462,7 +474,7 @@ function setObjectProperty(msg,prop,value,createMissing) { for (var i=0;i 1 && ((typeof obj[key] !== "object" && typeof obj[key] !== "function") || obj[key] === null)) { // Break out early as we cannot create a property beneath // this type of value @@ -561,7 +573,7 @@ function getSetting(node, name, flow_) { * @memberof @node-red/util_util */ function evaluateEnvProperty(value, node) { - var flow = (node && node.hasOwnProperty("_flow")) ? node._flow : null; + var flow = (node && hasOwnProperty.call(node, "_flow")) ? node._flow : null; var result; if (/^\${[^}]+}$/.test(value)) { // ${ENV_VAR} @@ -785,7 +797,7 @@ function normaliseNodeTypeName(name) { function encodeObject(msg,opts) { try { var debuglength = 1000; - if (opts && opts.hasOwnProperty('maxLength')) { + if (opts && hasOwnProperty.call(opts, 'maxLength')) { debuglength = opts.maxLength; } var msgType = typeof msg.msg; @@ -795,7 +807,7 @@ function encodeObject(msg,opts) { if (msg.msg.name) { errorMsg.name = msg.msg.name; } - if (msg.msg.hasOwnProperty('message')) { + if (hasOwnProperty.call(msg.msg, 'message')) { errorMsg.message = msg.msg.message; } else { errorMsg.message = msg.msg.toString(); @@ -809,7 +821,7 @@ function encodeObject(msg,opts) { } } else if (msg.msg && msgType === 'object') { try { - msg.format = msg.msg.constructor.name || "Object"; + msg.format = constructorName(msg.msg) || "Object"; // Handle special case of msg.req/res objects from HTTP In node if (msg.format === "IncomingMessage" || msg.format === "ServerResponse") { msg.format = "Object"; @@ -836,7 +848,7 @@ function encodeObject(msg,opts) { length: msg.msg.length } } - } else if (msg.msg && msg.msg.constructor.name === "Set") { + } else if (constructorName(msg.msg) === "Set") { msg.format = "set["+msg.msg.size+"]"; msg.msg = { __enc__: true, @@ -845,7 +857,7 @@ function encodeObject(msg,opts) { length: msg.msg.size } needsStringify = true; - } else if (msg.msg && msg.msg.constructor.name === "Map") { + } else if (constructorName(msg.msg) === "Map") { msg.format = "map"; msg.msg = { __enc__: true, @@ -854,7 +866,7 @@ function encodeObject(msg,opts) { length: msg.msg.size } needsStringify = true; - } else if (msg.msg && msg.msg.constructor.name === "RegExp") { + } else if (constructorName(msg.msg) === "RegExp") { msg.format = 'regexp'; msg.msg = msg.msg.toString(); } @@ -904,25 +916,25 @@ function encodeObject(msg,opts) { if (value.length > debuglength) { value.data = value.data.slice(0,debuglength); } - } else if (value.constructor.name === "ServerResponse") { + } else if (constructorName(value) === "ServerResponse") { value = "[internal]" - } else if (value.constructor.name === "Socket") { + } else if (constructorName(value) === "Socket") { value = "[internal]" - } else if (value.constructor.name === "Set") { + } else if (constructorName(value) === "Set") { value = { __enc__: true, type: "set", data: Array.from(value).slice(0,debuglength), length: value.size } - } else if (value.constructor.name === "Map") { + } else if (constructorName(value) === "Map") { value = { __enc__: true, type: "map", data: Object.fromEntries(Array.from(value.entries()).slice(0,debuglength)), length: value.size } - } else if (value.constructor.name === "RegExp") { + } else if (constructorName(value) === "RegExp") { value = { __enc__: true, type: "regexp", @@ -974,7 +986,7 @@ function encodeObject(msg,opts) { if (e.name) { errorMsg.name = e.name; } - if (e.hasOwnProperty('message')) { + if (hasOwnProperty.call(e, 'message')) { errorMsg.message = 'encodeObject Error: ['+e.message + '] Value: '+util.inspect(msg.msg); } else { errorMsg.message = 'encodeObject Error: ['+e.toString() + '] Value: '+util.inspect(msg.msg); diff --git a/test/nodes/core/common/21-debug_spec.js b/test/nodes/core/common/21-debug_spec.js index ae0d9a48e..2931ea1e4 100644 --- a/test/nodes/core/common/21-debug_spec.js +++ b/test/nodes/core/common/21-debug_spec.js @@ -265,6 +265,38 @@ describe('debug node', function() { }); }); + it('should publish an object with no-prototype-builtins', function(done) { + const flow = [{id:"n1", type:"debug" }]; + helper.load(debugNode, flow, function() { + const n1 = helper.getNode("n1"); + const payload = Object.create(null); + payload.type = 'foo'; + websocket_test(function() { + n1.emit("input", {payload: payload}); + }, function(msg) { + JSON.parse(msg).should.eql([{ + topic:"debug", + data:{id:"n1",msg:'{"type":"foo"}',property:"payload",format:"Object",path:"global"} + }]); + }, done); + }); + }); + + it('should publish an object with overriden hasOwnProperty', function(done) { + const flow = [{id:"n1", type:"debug" }]; + helper.load(debugNode, flow, function() { + const n1 = helper.getNode("n1"); + websocket_test(function() { + n1.emit("input", {payload: {type:'foo', hasOwnProperty: null}}); + }, function(msg) { + JSON.parse(msg).should.eql([{ + topic:"debug", + data:{id:"n1",msg:'{"type":"foo","hasOwnProperty":null}',property:"payload",format:"Object",path:"global"} + }]); + }, done); + }); + }); + it('should publish an array', function(done) { var flow = [{id:"n1", type:"debug" }]; helper.load(debugNode, flow, function() { diff --git a/test/unit/@node-red/util/lib/util_spec.js b/test/unit/@node-red/util/lib/util_spec.js index 5007fe5c5..2013f143e 100644 --- a/test/unit/@node-red/util/lib/util_spec.js +++ b/test/unit/@node-red/util/lib/util_spec.js @@ -830,6 +830,24 @@ describe("@node-red/util/util", function() { resultJson.b.should.have.property("__enc__", true); resultJson.b.should.have.property("type", "undefined"); }); + it('object with no prototype builtins', function() { + const payload = new Object(null); + payload.c = 3; + var msg = { msg:{b:payload} }; + var result = util.encodeObject(msg); + result.format.should.eql("Object"); + var resultJson = JSON.parse(result.msg); + resultJson.should.have.property("b"); + resultJson.b.should.have.property("c", 3); + }); + it('object with overriden hasOwnProperty', function() { + var msg = { msg:{b:{hasOwnProperty:null}} }; + var result = util.encodeObject(msg); + result.format.should.eql("Object"); + var resultJson = JSON.parse(result.msg); + resultJson.should.have.property("b"); + resultJson.b.should.have.property("hasOwnProperty"); + }); it('object with Map property', function() { const m = new Map(); m.set("a",1); From 2e1e61dabe224ca1818788cd5903f879191d09ce Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Thu, 3 Feb 2022 16:32:51 +0100 Subject: [PATCH 3/3] Remove part of JSDoc --- packages/node_modules/@node-red/util/lib/util.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/node_modules/@node-red/util/lib/util.js b/packages/node_modules/@node-red/util/lib/util.js index a81030fb3..a6fec8f98 100644 --- a/packages/node_modules/@node-red/util/lib/util.js +++ b/packages/node_modules/@node-red/util/lib/util.js @@ -29,7 +29,6 @@ const { hasOwnProperty } = Object.prototype; /** * Safely returns the object construtor name. * @return {String} the name of the object constructor if it exists, empty string otherwise. - * @memberof @node-red/util_util */ function constructorName(obj) { // Note: This function could be replaced by optional chaining in Node.js 14+: