Socket.IO handshake module

For the server of which this is part, it makes a request to one of our web servers to validate whether the PHP Session ID is actually valid. The location this._options.url is a URL that, given a PHPSESSID as a cookie, will return the user ID that is attached.

One of the issues that has crept up is the CPU utilization spiking to 100%. While I have seen several solutions to this, one was that failure to call a callback function can cause it. After effectively disabling the following function (I had it simply do callback(null,1); for testing purposes), the CPU utilization issue stopped.

However, I do believe a contributing cause was that the nature of performing an httpd request might not be trapping all of the errors and possibilities and have modified it to look this like:

```
var http = require('http');
var url = require('url');

function AuthFunc(auth_opts){
this._options = auth_opts;
this.checkPHPSession = function(phpsessid, callback){
var options = {
host: url.parse(this._options.url).host,
port: url.parse(this._options.url).port || 80,
path: url.parse(this._options.url).pathname,
auth: url.parse(this._options.url).auth,
headers: {
'Cookie': this._options.cookie + '=' + phpsessid
}
};
var data = '';
http.get(options, function(response){
response.on('data', function(chunk) {
data += chunk;
}).on('end', function(){
try{
var arr = JSON.parse(data);
}catch(e){
callback(e, false);
}
callback(null, arr.uid);
}).on('close', function(){
callback(true, false);
}).on('error', function(e){
callback(e, false);
});
}).on('error', function(e){
callback(e, false);
});
}
}

module.exports =

Interesting question, I would rewrite

try{
    var arr = JSON.parse(data);
}catch(e){
    callback(e, false);
}
callback(null, arr.uid);

as

try{
    var arr = JSON.parse(data);
    callback(null, arr.uid);
}catch(e){
    callback(e, false);
}

Actually, I would probably go even a step further and write it as

try{
    callback(null, JSON.parse(data).uid);
}catch(e){
    callback(e, false);
}

Otherwise in case of an error, you will have 2 callbacks, which could cause all kinds of trouble, perhaps even make the CPU spike to 100% ;)

Furthermore, I would also put a try/catch around http.get itself, if there is a hardcore http layer problem, then you might not even get to 'close' or 'error'.

Then some other minor items are

• Most of the options could be set outside of checkPHPSession, you should do so and prevent the repeated determinations of host, port, etc.

• Why are you checking for 'close', it seems like one more way to potentially have too many calls to the callback function

StackExchange Code Review Q#51300, answer score: 2