Referer undefined validation - in requestOptions

[hitman401]

If the referer is undefined in the requestOption headers, the plugin terminates with a crash. Tested in windows 7, 64 bit with nodejs 0.12.*.

Added validation for the same.

[cgiffard]

Can you provide a stack trace for the crash? I'd prefer to fix the crash itself than falsely inject a blank referrer value. (The undefined value can more accurately be used to determine that a referrer was not sent—as opposed to a blank referrer, which could be a header with no value.)

[hitman401]

Actually, it throws an error stating
Fatal error: "name" and "value" are required for setHeader()
And it breaks at this line,
// Get the resource! clientRequest = client.request(requestOptions,function(response) { crawler.handleResponse(queueItem,response,timeCommenced); });

exactly where it invokes client.request

As you rightly said, sending an empty string wouldn't be a good way. Sorry for that. But will it be fine to remove the referrer if it is undefined? If fine then I can submit one for the same.

[cgiffard]

I think the right solution would be to conditionally assign the referrer header, like so:

if (queueItem.referrer) {
    requestOptions.headers.Referer = queueItem.referrer;
}

That should stop the crash. Then we'll need to add a regression test for it!

[hitman401]

Thanks cgiffard!

I did make the change and also tested the fix from my end. It works fine now. Have updated the commit either. May be it helps now

[cgiffard]

Thanks Krishna!

In order to ensure that I (or another contributor) don't accidentally reintroduce this bug, would you be able to assist by providing a regression test? I am happy to help you with this if required.

[hitman401]

Sure cgiffard,

Let me know the process and I will try my best to help you with this. If you can point me, in what is expected from me, then I can definitely help you on this.

[pose]

I've added a test here: #111 . @cgiffard would you mind releasing a new version after this fix? Thanks!