javascript property extractor optimisation

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"){

} // 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


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.
• I check for shift method to avoid TypeError and return rescue value in case if path is meaningless. And path as a string is merely syntactic sugar. – ts01 Mar 20 '12 at 21:27
• @ts01 But it's not really useful to get the rescue value, if shift doesn't exist (or is the wrong type). It would be much better to A) clearly document that it's required and B) throw an exception. Alternatively you should either implement it yourself, if it's so important, or avoid using it all. – RoToRa Mar 21 '12 at 8:23
• @seand Thanks for the edit. I wrote that on my phone which doesn't allow me to enter the backtick. – RoToRa Mar 21 '12 at 8:28

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.

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


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