From 8b74ed9f1b0261ec99fbd3fb0cc212a1e953aece Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 19 Jan 2013 19:54:48 -0500 Subject: [PATCH] Corrected a bug whereby getters were being inadvertently invoked by util.propParse() Nasty; hopefully this was found before it did any harm to anyone else! This bug was discovered accidentally while I was debugging a separate issue. --- lib/util.js | 6 +++++- test/test-util-prop-parse.js | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/util.js b/lib/util.js index 34c0fb6..43cd6e8 100644 --- a/lib/util.js +++ b/lib/util.js @@ -297,7 +297,11 @@ exports.propParse = function( data, options ) setter = prop_desc.set; } - value = data[ prop ]; + // do not attempt to retrieve the value if a getter is defined (as that + // would then call the getter) + value = ( typeof getter === 'function' ) + ? undefined + : data[ prop ]; parse_data = keywordParser( prop ) || {}; name = parse_data.name || prop; diff --git a/test/test-util-prop-parse.js b/test/test-util-prop-parse.js index 6ca81df..7d4fd79 100644 --- a/test/test-util-prop-parse.js +++ b/test/test-util-prop-parse.js @@ -51,11 +51,13 @@ var data = { 'proxy someProxy': 'dest', }; +get_called = false; + // only add getter/setter if it's supported by our engine if ( get_set ) { Object.defineProperty( data, 'someFoo', { - get: function () {}, + get: function () { get_called = true; }, set: function () {}, enumerable: true, @@ -80,8 +82,10 @@ util.propParse( data, { // run for each item in data each: function( name, value ) { - // only remove if the passed value is correct - if ( value === data[ name ] ) + // only remove if the passed value is correct (note the check for + // 'someFoo', since this has a getter and checking its value would + // invoke the getter, which would taint one of the tests) + if ( ( name === 'someFoo' ) || ( value === data[ name ] ) ) { delete chk_each[ name ]; } @@ -158,6 +162,11 @@ if ( get_set ) data.__lookupSetter__( 'someFoo' ), "Property parser properly detects setters" ); + + // bug fix + assert.equal( false, get_called, + "Getter should not be called during processing" + ); }