Read path for cookies

[kitek]

Changed path to Path (http://en.wikipedia.org/wiki/HTTP_cookie) in vendor/cookie/index.js:43-44
This patch fixes bug: TypeError: Cannot read property 'url' of undefined

Code to reproduce error:

request('https://bramka.play.pl/composer/public/mmsCompose.do', function (error, response, body) {
    if (!error && response.statusCode == 200) {
        console.log(body);
    }
});

[janjongboom]

It's useless to do req.url there because req is always NULL. Rather do

this.Path || "/"

Furthermore: there are websites sending path with a lower case 'p', so we need something more robust here; something like:

 (Object.keys(this).filter(function (k) { return !!k.match(/path/i); }) || [ "/" ])[0]

[jhurliman]

+1 for getting this fixed. I recently saw:

TypeError: Cannot read property 'url' of undefined
   at new Cookie
(.../node_modules/request/vendor/cookie/index.js:45:20)
   at .../node_modules/request/main.js:432:33
   at Array.forEach (native)
   at Request.<anonymous>
(.../node_modules/request/main.js:426:46)
   at Request.emit (events.js:64:17)
   at IncomingMessage.<anonymous>
(.../node_modules/request/main.js:391:16)
   at IncomingMessage.emit (events.js:81:20)
   at HTTPParser.onMessageComplete (http.js:133:23)
   at Socket.onend (http.js:1269:12)
   at Socket._onReadable (net.js:653:26)

[janjongboom]

@kitek, looks good, furthermore also a +1 for this to be fixed, have this quite often.

[Filirom1]

@kitek Good job, you fixed the issue.

When will it be merged ?

You can reproduce it easily

request({url: 'http://www.ticketnet.fr/Resultat?ipSearch=lyon'}, function(){ console.log(arguments) })

[donpark]

+1 for this patch. This issue renders jsdom useless for crawling.

[phstc]

• 1 for this patch. It worked very well. @kitek tks.

[toabi]

+1, please merge quickly so I don't have to tell everybody in my team how to change their libs by hand ;)

[avanderhoorn]

+1 I have the same problem when hitting different sites.

[phstc]

How to apply this patch on Heroku?

Heroku doesn't read the node_modules folder. :/

[SOLUTION]

To use this patch on Heroku, I moved /node_modules/request to /request and then I changed the require

from

  require('request')

to

  require('./request')

[LinusU]

I propose the following change, making it read the key as lower case all the time. At the moment it seems like it is expecting "path" to be in lower-case when in fact the RFC (http://www.ietf.org/rfc/rfc2109.txt) says it should be with a capital P.

I do however think that it would be smart to change all the keys to lower case to ensure maximum compatibility with different web-servers.

Line 27 - 32:

// Map the key/val pairs
str.split(/ *; */).reduce(function(obj, pair){
  pair = pair.split(/ *= */);
  obj[pair[0].toLowerCase()] = pair[1] || true;
  return obj;
}, this);

The change is from pair[0] to pair[0].toLowerCase() on line 30.

[jhurliman]

I don't know how the request library is still usable for anyone with this regression. As a temporary measure, I've forked the library and applied the patches suggested above. If you are using request directly in a project, you can change your package.json to use the forked copy of the lib like this:

"dependencies": {
  ...
  "request": "git://github.com/jhurliman/request.git",
  ...
}

[mikeal]

this doesn't merge cleanly

[LinusU]

Well, I guess we don't need this one since #121 got merged, or am I missing something?

[mikeal]

closing, i think this is already fixed.