Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle cookies #82

Closed
juggy opened this issue Sep 27, 2011 · 15 comments
Closed

Handle cookies #82

juggy opened this issue Sep 27, 2011 · 15 comments

Comments

@juggy
Copy link

juggy commented Sep 27, 2011

I stumbled upon a page that set cookies in the redirection. I don't know if it is part of the http spec or not, but I had to change the redirection code within request to handle this.

Since request does not handle cookies itself, it might make sense to build a generic callback where the caller can play with the headers etc.

Thanks,
Julien.

Mikeal changed the title to add more generic support for cookies.

@mikeal
Copy link
Member

mikeal commented Sep 27, 2011

What I really want is an option that just sets and handles cookies the way the browser does.

Unfortunately the existing cookie libraries weren't working for me and I had to put that feature on the back burner.

-Mikeal

On Sep 27, 2011, at September 27, 20114:22 AM, Julien Guimont wrote:

I stumbled upon a page that set cookies in the redirection. I don't know if it is part of the http spec or not, but I had to change the redirection code within request to handle this.

Since request does not handle cookies itself, it might make sense to build a generic callback where the caller can play with the headers etc.

Thanks,
Julien.

Reply to this email directly or view it on GitHub:
#82

@juggy
Copy link
Author

juggy commented Sep 27, 2011

What libraries did you look at?

@Pita
Copy link

Pita commented Sep 29, 2011

+1

@waynelee
Copy link

juggy -- care to share your cookie changes to request? i'll take hacks!

+1 on callback support to twiddle with headers or cookies support.

@waynelee
Copy link

Here's a patch that adds cookie support. Warning, js and node.js and git and github and http newbie here. It works for my application, but no idea if it is per spec. Depends on node-cookiejar, which means package.json should be updated.

From 4bf36075d354dd5c41f1598249bf05a161da0cc2 Mon Sep 17 00:00:00 2001
From: Wayne Lee waynelee@wal-imac.local
Date: Tue, 11 Oct 2011 23:27:44 -0700
Subject: [PATCH] Add cookie support in redirects


main.js | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/main.js b/main.js
index de75caf..28ef534 100644
--- a/main.js
+++ b/main.js
@@ -263,6 +263,14 @@ Request.prototype.request = function () {
self.uri = response.headers.location
self.redirects.push( { statusCode : response.statusCode,
redirectUri: response.headers.location })
+

  •    if (self.response.headers['set-cookie']) {
    
  •        var cj = require('cookiejar').CookieJar()
    
  •        cj.setCookies(response.headers['set-cookie'])
    
  •        cookies = cj.getCookies(CookieAccessInfo(url.parse(self.uri).host))
    
  •        self.headers['Cookie'] = cookies.toValueString()
    
  •    }
    
    • delete self.req
      delete self.agent
      delete self._started
      --
      1.7.4.4

@juggy
Copy link
Author

juggy commented Oct 13, 2011

@waynelee Good but you might do the initial request with cookies, use those cookies for the redirection and, finally, se them after the redirection. My code is similar but you have a callback when the redirection occurs where you can do whatever you want (including cookies).

@mikeal
Copy link
Member

mikeal commented Nov 10, 2011

this is the API i want for cookies

var j = request.jar()
r({url:'http://www.google.com', jar:j}, function () {
  r({url:'http://images.google.com', jar:j})
})

The jar holds the state of the cookies being set, each request sends the proper cookies for that domain. The boiler is easy to get rid of with defaults.

var j = request.jar()
var r = request.defaults({jar:j})
request('http://www.google.com', function () {
  request('http://images.google.com')
})

@Marak
Copy link

Marak commented Nov 10, 2011

It looks like https://github.com/bmeck/node-cookiejar might be a good candidate for this API?

@alessioalex - Do you want to try looking into this and seeing what you can do?

@mikeal
Copy link
Member

mikeal commented Nov 10, 2011

if we go with node-cookie jar please check the dependency. i do not want to add dependencies to the package.json if at all avoidable.

@alessioalex
Copy link

I'll take a look over the weekend on https://github.com/bmeck/node-cookiejar/blob/master/cookiejar.js + https://github.com/LearnBoost/tobi/blob/master/lib/cookie/jar.js and try to come up with a useful implementation

@juggy
Copy link
Author

juggy commented Nov 12, 2011

I think you can solve this without actually adding any specific code for cookies. When you do your request/redirect, a callback can be called to let the user deal with the headers/cookies. No new dependencies would be needed and it would be a little more future proof.

As a user of request, if I need cookies, I use any of the available libs to do that (or deal with them myself). A wiki page or a code exemple should be available to ease the implementation.

What do you think?

@Pita
Copy link

Pita commented Nov 12, 2011

@juggy I did this, its a pain

@alessioalex
Copy link

I have this almost 90% implemented, no dependencies needed, I'll finish it in one hour or so and then make a pull request. Bear with me :)

@lbdremy
Copy link

lbdremy commented Jan 24, 2012

What's up about this issue, guys? I have the same problem.

@mikeal
Copy link
Member

mikeal commented Jan 25, 2012

this got merged forever ago.

@mikeal mikeal closed this as completed Jan 25, 2012
wangyufish pushed a commit to IbpTeam/node-request that referenced this issue Sep 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants