javascript property extractor optimisation

Question

This is a very simple nested property extractor. I wonder if it can be optimised further?

/**
 * Extract nested property from object. If nested property cannot be reached, return value of rescue
 * @param obj Object
 * @param path Can be dot-separated  string or array
 * @param rescue (optional) default value. Defaults to null
 */
function extract(obj, path, rescue){

    if (typeof obj === "object" && path){

        var elements = typeof path === "string" ? path.split(".") : path;

        if (typeof elements.shift === "function"){

             var head = elements.shift();

             if (obj.hasOwnProperty(head)){

                return (elements.length === 0) ? obj[head] : extract(obj[head], elements, rescue);

             } // if

         } // if

    } // if

    return rescue || null;    

} // extract


var noob = {k1 : {k11 : {k111 : "v1"}}, k2 : { k21 : "v2"}};

console.log(extract(noob, 'k1.k11')); // {k111 : "v1"}
console.log(extract(noob, 'k1.k11.k111')); // v1
console.log(extract(noob, ['k1', 'k11', 'k111'])); // v1
console.log(extract(noob, 'k1.k11.kx')); // null
console.log(extract(noob, 'k2')); //  k2 : { k21 : "v2"}
console.log(extract(noob, 'k2.k21.k22')); // null
console.log(extract(noob, 'k1.k11.k22', "ZUT")); // ZUT     
console.log(extract(noob, '', "ZUT"));  // ZUT          
console.log(extract(false, '', "ZUT"));  // ZUT   

​

Answer 1

Here are some of my thoughts:

• I don't see the point in providing the path as a dot separated string, but it may be useful in your application.
• If you keep the dot separated string, then you should consider extracting the recursive function into a separate internal function, so that you don't have to repeat the check of the path argument in each iteration.
• Why do you check for the shift method? If it doesn't exist, then your function will fail silently. The regular "method does not exist" exception would be much more useful. Instead implement the method on the Array.prototype yourself separately if it doesn't exist.
• Finally I would move the path.length == 0 to the start of the function. That is where the break condition of recursive functions are usually expected.

Answer 2

RoToRa is absolutely correct. A few other considerations:

• A dot is a valid property name part. Whatcha gonna do when:

  var x = {}; x["blah.blam"] = "bloo";

• Why are you checking hasOwnProperty? This would exclude any usage of prototype inheritance.

For the above reasons I would recommend against using this as a general utility function. However, if this is going to be a specific utility (for example you're trying to create a simple data-binding framework where you know you won't have to worry about the above) this might be the rare legitimate use of the controversial with statement.

var x = {
  blah: {
     blam: "bloo"
  }
}
with(x) {
  console.log(blah)
}
with(x){
  console.log(blah.blam)
}

To take you the rest of the way you use the even more maligned eval:

var extract = function(obj, path, rescue){
  with(obj) {
    return eval(path) || rescue;
  }
}

Yes this doesn't do all the type checks that you do above but why do you need them?

Now let me be clear

It is possible to expose an XSS vulnerability for your users here

Specifically if you allow users to enter values which are posted back, persisted on the server, downloaded by other users, and then used with this function on their computers.

Suppose you are doing binding to objects which users can customize. These can be arbitrarily nested so you use this pattern. You also have summary screens in which users can view other users' customizations.

All an attacker has to do is create a property named some javascript code that steals browser information and it will be eval'ed and run on the machines of other users of the system.

That's the danger. If you're aware of it and make sure that condition never happens, feel free to use the with-eval.

Answer 3

As suggested by others, the string approach with dot-separated properties can be ambigious. I present an alternative using lambda functions. It chains property extraction from inner objects, until a property can no longer be retrieved, in which case the default value is returned. It also performs an early exit earlier than the initial code.

function extract (source, selectors, defaultValue) {
  if (source == undefined || selectors == undefined) {
    return defaultValue;
  }
  if (!Array.isArray(selectors)) {
    selectors = [ selectors ];
  }
  let value = source;
  for (const i in selectors) {
    try {
      value = selectors[i](value);
    } catch {
      value = defaultValue;
      break;
    }
  }
  return value;
}

Usage:

const data = {
  inner: {
    message: 'Hello world',
  }
}

// 'Hello world'
var prop1 = extract(data, [
      obj => obj.inner
    , obj => obj.message
  ], 'not found');

// 'not found'
var prop2 = extract(data, [
      obj => obj.unknownpropname
    , obj => obj.message
  ], 'not found');

Your example reworked:

var noob = {k1 : {k11 : {k111 : "v1"}}, k2 : { k21 : "v2"}};

// 'v1'
var prop = extract(noob, [obj => obj.k1, obj => obj.k11, obj => obj.k111 ], 'not found');