Skip to content

Commit

Permalink
Remove dangerous hasOwnProperty
Browse files Browse the repository at this point in the history
  • Loading branch information
NatalieWolfe committed Jun 22, 2017
1 parent ce4c17e commit a383b2e
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 37 deletions.
3 changes: 2 additions & 1 deletion api.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ var recordWeb = require('./lib/metrics/recorders/http.js')
var recordBackground = require('./lib/metrics/recorders/other.js')
var customRecorder = require('./lib/metrics/recorders/custom')
var hashes = require('./lib/util/hashes')
var properties = require('./lib/util/properties')
var stringify = require('json-stringify-safe')
var shimmer = require('./lib/shimmer.js')
var Shim = require('./lib/shim/shim.js')
Expand Down Expand Up @@ -286,7 +287,7 @@ API.prototype.addCustomParameters = function addCustomParameters(params) {
metric.incrementCallCount()

for (var key in params) {
if (!params.hasOwnProperty(key)) {
if (!properties.hasOwn(params, key)) {
continue
}

Expand Down
8 changes: 6 additions & 2 deletions lib/db/parsed-statement.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

var DB = require('../metrics/names').DB
var ALL = require('../metrics/names').ALL
var hasOwnProperty = require('../util/properties').hasOwn


function ParsedStatement(type, operation, model, raw) {
this.type = type
Expand Down Expand Up @@ -49,8 +51,10 @@ ParsedStatement.prototype.recordMetrics = function recordMetrics(segment, scope)
segment.name = model || operation

// Datastore instance metrics.
if (segment.parameters.hasOwnProperty('host') &&
segment.parameters.hasOwnProperty('port_path_or_id')) {
if (
hasOwnProperty(segment.parameters, 'host') &&
hasOwnProperty(segment.parameters, 'port_path_or_id')
) {
var instanceName = DB.INSTANCE + '/' + thisTypeSlash + segment.parameters.host +
'/' + segment.parameters.port_path_or_id
transaction.measure(instanceName, null, duration, exclusive)
Expand Down
3 changes: 2 additions & 1 deletion lib/instrumentation/core/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ var instrumentOutbound = require('../../transaction/tracer/instrumentation/outbo
var util = require('util')
var url = require('url')
var urltils = require('../../util/urltils')
var properties = require('../../util/properties')
var semver = require('semver')
var copy = require('../../util/copy')

Expand Down Expand Up @@ -64,7 +65,7 @@ function wrapEmitWithTransaction(agent, emit) {

// Hook for web framework instrumentations that don't have easy access to
// the request entry point.
if (this.hasOwnProperty('__NR_onRequestStarted')) {
if (properties.hasOwn(this, '__NR_onRequestStarted')) {
this.__NR_onRequestStarted(request, response)
}

Expand Down
4 changes: 3 additions & 1 deletion lib/instrumentation/mysql.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict'

var dbutils = require('../db/utils')
var properties = require('../util/properties')


module.exports = function initialize(agent, mysql, moduleName, shim) {
shim.setDatastore(shim.MYSQL)
Expand Down Expand Up @@ -189,7 +191,7 @@ function getInstanceExtras(shim, queryable, query) {
if (conf) {
extras.database_name = databaseName = databaseName || conf.database

if (conf.hasOwnProperty('socketPath') && conf.socketPath) {
if (properties.hasOwn(conf, 'socketPath') && conf.socketPath) {
// In the unix domain socket case we force the host to be localhost
extras.host = 'localhost'
extras.port_path_or_id = conf.socketPath
Expand Down
5 changes: 3 additions & 2 deletions lib/instrumentation/promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var logger = require('../logger')
var util = require('util')
var properties = require('../util/properties')
var shimmer = require('../shimmer')


Expand Down Expand Up @@ -109,7 +110,7 @@ module.exports = function initialize(agent, library, spec) {
var passThrough = spec.$static && spec.$static.$passThrough
if (passThrough) {
passThrough.forEach(function assignProxy(proxyProp) {
if (!wrappedPromise.hasOwnProperty(proxyProp)) {
if (!properties.hasOwn(wrappedPromise, proxyProp)) {
Object.defineProperty(wrappedPromise, proxyProp, {
enumerable: true,
configurable: true,
Expand Down Expand Up @@ -523,7 +524,7 @@ function _setInternalProperty(obj, name, val) {
}

try {
if (!obj.hasOwnProperty(name)) {
if (!properties.hasOwn(obj, name)) {
Object.defineProperty(obj, name, {
enumerable: false,
writable: true,
Expand Down
11 changes: 6 additions & 5 deletions lib/instrumentation/redis.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

var hasOwnProperty = require('../util/properties').hasOwn
var stringifySync = require('../util/safe-json').stringifySync

module.exports = function initialize(agent, redis, moduleName, shim) {
Expand Down Expand Up @@ -70,16 +71,16 @@ module.exports = function initialize(agent, redis, moduleName, shim) {
}

function getInstanceExtras(client) {
if (client.hasOwnProperty('port') && client.hasOwnProperty('host')) {
if (hasOwnProperty(client, 'port') && hasOwnProperty(client, 'host')) {
// for redis <=0.11
return doCapture(client)
} else if (client.hasOwnProperty('connection_options')) {
} else if (hasOwnProperty(client, 'connection_options')) {
// for redis 2.4.0 - 2.6.2
return doCapture(client.connection_options)
} else if (client.hasOwnProperty('connectionOption')) {
} else if (hasOwnProperty(client, 'connectionOption')) {
// for redis 0.12 - 2.2.5
return doCapture(client.connectionOption)
} else if (client.hasOwnProperty('options')) {
} else if (hasOwnProperty(client, 'options')) {
// for redis 2.3.0 - 2.3.1
return doCapture(client.options)
}
Expand All @@ -92,7 +93,7 @@ module.exports = function initialize(agent, redis, moduleName, shim) {
}

function doCapture(opts) {
var db = (client.hasOwnProperty('selected_db') ? client.selected_db : opts.db) || 0
var db = (hasOwnProperty(client, 'selected_db') ? client.selected_db : opts.db) || 0

return {
host: opts.host || 'localhost',
Expand Down
7 changes: 5 additions & 2 deletions lib/metrics/recorders/memcached.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
var NAMES = require('../names')
var DB = NAMES.DB
var MEMCACHE = NAMES.MEMCACHE
var properties = require('../../util/properties')


function recordMemcache(segment, scope) {
Expand All @@ -22,8 +23,10 @@ function recordMemcache(segment, scope) {
transaction.measure(MEMCACHE.ALL, null, duration, exclusive)

// Datastore instance metrics.
if (segment.parameters.hasOwnProperty('host') &&
segment.parameters.hasOwnProperty('port_path_or_id')) {
if (
properties.hasOwn(segment.parameters, 'host') &&
properties.hasOwn(segment.parameters, 'port_path_or_id')
) {
var instanceName =
DB.INSTANCE + '/' + MEMCACHE.PREFIX + '/' + segment.parameters.host + '/' +
segment.parameters.port_path_or_id
Expand Down
7 changes: 5 additions & 2 deletions lib/metrics/recorders/redis.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
var NAMES = require('../names')
var DB = NAMES.DB
var REDIS = NAMES.REDIS
var properties = require('../../util/properties')


function recordRedis(segment, scope) {
Expand All @@ -22,8 +23,10 @@ function recordRedis(segment, scope) {
transaction.measure(REDIS.ALL, null, duration, exclusive)

// Datastore instance metrics.
if (segment.parameters.hasOwnProperty('host') &&
segment.parameters.hasOwnProperty('port_path_or_id')) {
if (
properties.hasOwn(segment.parameters, 'host') &&
properties.hasOwn(segment.parameters, 'port_path_or_id')
) {
var instanceName =
DB.INSTANCE + '/' + REDIS.PREFIX + '/' + segment.parameters.host + '/' +
segment.parameters.port_path_or_id
Expand Down
18 changes: 10 additions & 8 deletions lib/shim/datastore-shim.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

var hasOwnProperty = require('../util/properties').hasOwn
var logger = require('../logger.js').child({component: 'DatastoreShim'})
var metrics = require('../metrics/names')
var parseSql = require('../db/query-parsers/sql')
Expand All @@ -8,6 +9,7 @@ var Shim = require('./shim')
var urltils = require('../util/urltils')
var util = require('util')


/**
* An enumeration of well-known datastores so that new instrumentations can use
* the same names we already use for first-party instrumentation.
Expand Down Expand Up @@ -438,12 +440,12 @@ function recordOperation(nodule, properties, opSpec) {
record: opSpec.record
}
}
if (segDesc.hasOwnProperty('extras')) {
if (hasOwnProperty(segDesc, 'extras')) {
_normalizeExtras.call(shim, segDesc.extras)
}

// Adjust the segment name with the metric prefix and add a recorder.
if (!segDesc.hasOwnProperty('record') || segDesc.record !== false) {
if (!hasOwnProperty(segDesc, 'record') || segDesc.record !== false) {
segDesc.name = shim._metrics.OPERATION + segDesc.name
segDesc.recorder = _recordOperationMetrics.bind(shim)

Expand Down Expand Up @@ -707,7 +709,7 @@ function _recordQuery(suffix, nodule, properties, querySpec) {

// If we're not actually recording this, then just return the segment
// descriptor now.
if (queryDesc.hasOwnProperty('record') && queryDesc.record === false) {
if (hasOwnProperty(queryDesc, 'record') && queryDesc.record === false) {
return {
name: queryDesc.name || fnName,
extras: _normalizeExtras.call(shim, queryDesc.extras || {}),
Expand Down Expand Up @@ -790,8 +792,8 @@ function _recordOperationMetrics(segment, scope) {
)

if (
segment.parameters.hasOwnProperty('host') &&
segment.parameters.hasOwnProperty('port_path_or_id')
hasOwnProperty(segment.parameters, 'host') &&
hasOwnProperty(segment.parameters, 'port_path_or_id')
) {
var instanceName =
metrics.DB.INSTANCE + '/' + this._metrics.PREFIX + '/' +
Expand Down Expand Up @@ -855,7 +857,7 @@ function _normalizeExtras(extras) {
// Add database name if provided and enabled.
if (!dsTracerConf.database_name_reporting.enabled) {
delete extras.database_name
} else if (extras.hasOwnProperty('database_name') && extras.database_name !== false) {
} else if (hasOwnProperty(extras, 'database_name') && extras.database_name !== false) {
extras.database_name = typeof extras.database_name === 'number'
? String(extras.database_name)
: (extras.database_name || INSTANCE_UNKNOWN)
Expand All @@ -867,10 +869,10 @@ function _normalizeExtras(extras) {
delete extras.port_path_or_id
} else {
// Determine appropriate defaults for host and port.
if (extras.hasOwnProperty('port_path_or_id')) {
if (hasOwnProperty(extras, 'port_path_or_id')) {
extras.port_path_or_id = String(extras.port_path_or_id || INSTANCE_UNKNOWN)
}
if (extras.hasOwnProperty('host')) {
if (hasOwnProperty(extras, 'host')) {
if (extras.host && urltils.isLocalhost(extras.host)) {
extras.host = config.getHostnameSafe(extras.host)
}
Expand Down
16 changes: 8 additions & 8 deletions lib/shim/shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var arity = require('../util/arity')
var events = require('events')
var hasOwnProperty = require('../util/properties').hasOwn
var logger = require('../logger.js').child({component: 'Shim'})
var path = require('path')
var util = require('util')
Expand Down Expand Up @@ -1269,13 +1270,13 @@ function createSegment(name, recorder, parent) {
segment.internal = opts.internal
segment.shim = this

if (opts.hasOwnProperty('extras')) {
if (hasOwnProperty(opts, 'extras')) {
var ignoredParams = this.agent.config.ignored_params
var captureParams = this.agent.config.capture_params
for (var key in opts.extras) {
if (
opts.extras.hasOwnProperty(key) && (
IGNORES_CAPTURE_PARAMS.hasOwnProperty(key) ||
hasOwnProperty(opts.extras, key) && (
hasOwnProperty(IGNORES_CAPTURE_PARAMS, key) ||
(captureParams && ignoredParams.indexOf(key) === -1)
)
) {
Expand Down Expand Up @@ -1511,7 +1512,7 @@ function setInternalProperty(obj, name, val) {
}

try {
if (!obj.hasOwnProperty(name)) {
if (!hasOwnProperty(obj, name)) {
Object.defineProperty(obj, name, {
enumerable: false,
writable: true,
Expand Down Expand Up @@ -1591,12 +1592,11 @@ function setDefaults(obj, defaults) {
if (!obj) {
obj = {}
}
var hasOwnProperty = Object.hasOwnProperty
var keys = Object.keys(defaults)

for (var i = 0; i < keys.length; ++i) {
var key = keys[i]
if (!hasOwnProperty.call(obj, key)) {
if (!hasOwnProperty(obj, key)) {
obj[key] = defaults[key]
}
}
Expand Down Expand Up @@ -1842,12 +1842,12 @@ function _makeBindWrapper(shim, fn, segment, full) {
*/
function _bindAllCallbacks(shim, fn, name, args, spec) {
// Check for a normal callback.
if (spec.spec.hasOwnProperty('callback') && spec.spec.callback !== null) {
if (hasOwnProperty(spec.spec, 'callback') && spec.spec.callback !== null) {
_bindCallback(this, spec.spec.callback, shim.bindCallbackSegment)
}

// And check for a row callback.
if (spec.spec.hasOwnProperty('rowCallback') && spec.spec.rowCallback !== null) {
if (hasOwnProperty(spec.spec, 'rowCallback') && spec.spec.rowCallback !== null) {
_bindCallback(
this,
spec.spec.rowCallback,
Expand Down
6 changes: 4 additions & 2 deletions lib/shimmer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ var path = require('path')
var fs = require('fs')
var logger = require('./logger').child({component: 'shimmer'})
var INSTRUMENTATION = require('./instrumentations')()
var properties = require('./util/properties')
var shims = require('./shim')


/*
*
* CONSTANTS
Expand Down Expand Up @@ -72,7 +74,7 @@ var instrumented = []
*/
function instrument(agent, nodule, moduleName, resolvedName) {
var instrumentation = registeredInstrumentations[moduleName]
if (nodule.hasOwnProperty('__NR_instrumented')) {
if (properties.hasOwn(nodule, '__NR_instrumented')) {
logger.trace(
'Already instrumented %s, skipping redundant instrumentation',
moduleName
Expand All @@ -81,7 +83,7 @@ function instrument(agent, nodule, moduleName, resolvedName) {
}

var shim = null
if (SHIM_TYPE_MAP.hasOwnProperty(instrumentation.type)) {
if (properties.hasOwn(SHIM_TYPE_MAP, instrumentation.type)) {
var ShimClass = SHIM_TYPE_MAP[instrumentation.type]
shim = new ShimClass(agent, moduleName, resolvedName)
} else {
Expand Down
4 changes: 3 additions & 1 deletion lib/uninstrumented.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ var path = require('path')
var logger = require('./logger')
var NAMES = require('./metrics/names')
var INSTRUMENTATIONS = require('./instrumentations')()
var properties = require('./util/properties')


module.exports = {
check: check,
Expand Down Expand Up @@ -71,7 +73,7 @@ function moduleNameFromFilename(filename) {
// they are missing instrumentation for core modules.
function check() {
for (var filename in require.cache) {
if (!Object.hasOwnProperty.call(require.cache, filename)) {
if (!properties.hasOwn(require.cache, filename)) {
continue
}
var name = moduleNameFromFilename(filename)
Expand Down
3 changes: 2 additions & 1 deletion lib/util/arity.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

var hasOwnProperty = require('./properties').hasOwn
var semver = require('semver')


Expand All @@ -14,7 +15,7 @@ function _fixArity_prop(original, wrapper) {
length: {value: original.length},
name: {value: original.name}
})
if (!wrapper.hasOwnProperty('__NR_name')) {
if (!hasOwnProperty(wrapper, '__NR_name')) {
Object.defineProperty(wrapper, '__NR_name', {
configurable: false,
enumerable: false,
Expand Down
16 changes: 16 additions & 0 deletions lib/util/properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@

var hasOwnProperty = Object.hasOwnProperty

/**
* Checks if an object has its own property with the given key.
*
* It is possible to create objects which do not inherit from `Object` by doing
* `Object.create(null)`. These objects do not have the `hasOwnProperty` method.
* This method uses a cached version of `hasOwnProperty` to check for the
* property, thus avoiding the potential `undefined is not a function` error.
*
* @private
*
* @param {*} obj - The item to check for the property on.
* @param {string} key - The name of the property to look for.
*
* @return {bool} True if the given object has its own property with the given
* key.
*/
exports.hasOwn = function hasOwn(obj, key) {
return hasOwnProperty.call(obj, key)
}
Loading

0 comments on commit a383b2e

Please sign in to comment.