code review 5399043: net/http: Added interface for a cookie jar.

net/http: Added interface for a cookie jar.

Types implementing CookieJar may be used in a Client
to persist cookies.

[volker.dobler, 2011-11-16 10:25:10]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[volker.dobler, 2011-11-16 12:45:45]

http: Handling cookies while following redirects: Proposal for a cookie jar.

This is a proposal for a CookieJar interface, some sample
types implementing this interface and how these could be
added to Client so that cookies are not lost while following
redirects.

Please take a look at the design, implementation is
not tested jet (it's just to check if the design is utterly
broken).  I'am not sure if Client is the perfect place
for a cookie jar, or even if a full fletched jar is
needed/wanted at all.

I started work on this as I needed fine control over
which cookies are set/deleted/sent during a redirect chain
and thought this might be usefull.

Any comments welcome.

[volker.dobler, 2011-12-01 13:33:15]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

Please take another look.

[volker.dobler, 2011-12-01 13:40:44]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2011-12-01 18:14:33]

Cool. This is basically the design I was hoping to see. Thanks for doing
this.

I'll reply with code + doc + naming comments in a bit.


On Thu, Dec 1, 2011 at 5:33 AM, <dr.volker.dobler@gmail.com> wrote:

> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
http://codereview.appspot.com/**5399043/<http://codereview.appspot.com/5399043/>
>

[bradfitz, 2011-12-01 21:12:06]

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:43: // nil, than each GET and HEAD request gets it's
own
its, not it's

But I think we want the nil case to be a BlackHoleJar.  And I'm not sure I'd
expose BlackHoleJar publicly.  I'd just say "nil means cookies aren't
supported".

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:190: var jar CookieJar
jar := c.Jar
if jar == nil {
    jar = blackHoleJar{}
}

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:23: Update(cookies []*Cookie, host string)
What about https vs. not?  I think instead of "host string" we might want "url
*url.URL" for the URL that sent us the cookies.

And I'd put that argument first.

And I'd name the methods something like:

StoreCookies
GetCookies

But I'm open to alternate proposals.  (Russ?)

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:33: type BlackHoleJar int
type blackHoleJar struct{}

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:35: func (bh BlackHoleJar) Update(cookies []*Cookie,
host string) {}
Don't need receiver names here:

func (blackHoleJar) Foo() ...

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:206: type TrivialJar struct {
Can we drop this implementation from this CL for now and focus just on the
Client and interface API changes?

We can add this back in a subsequent CL, perhaps named differently too
(MemoryJar? who knows).  Trivial things become less trivial over time.  This is
really about storing them in memory, I believe.  Also, the cookie policy logic
should be separate from the cookie storing/loading policy.  In this
implementation they're kinda combined.

The implementations will need tests too.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:260: // SimpleCookieJar implements a simple cookie jar.
It can be shared by different
Let's drop this one too.  I wouldn't have two anyway.  Let's just have a
thread-safe one (later).

I would even document in the CookieJar interface that the implementations must
be safe for use by concurrent goroutines.

[volker.dobler, 2011-12-01 21:35:15]

Thanks for the comments.  Maybe blackHoleJar
is not needed at all as there are just two uses
of the jar in client.go.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:190: var jar CookieJar
On 2011/12/01 21:12:06, bradfitz wrote:
> jar := c.Jar
> if jar == nil {
>     jar = blackHoleJar{}
> }

If nil means: No cookies during redirects, than
I would just check this case twice below. I.e.
here just:

jar := c.Jar

...

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:219: }
... and here
if jar != nil {
    for _, cookie := range jar.Select(req.URL) {
        req.AddCookie(cookie)
    }
}
...

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:229: jar.Update(r.Cookies(), hostname(req.URL))
... and here once more:

if jar != nil { jar.Update(r.Cookies(), hostname(req.URL)) }

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:23: Update(cookies []*Cookie, host string)
On 2011/12/01 21:12:06, bradfitz wrote:
> What about https vs. not?  I think instead of 
> "host string" we might want "url *url.URL" for 
> the URL that sent us the cookies.

I must admit, that I just do not know whether
it should be allowed to set a Secure cookie from
a https request and vice versa.  I did not come
up with any reasons why it should be forbidden.
That is why I just used host instead of the whole
URL.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:33: type BlackHoleJar int
On 2011/12/01 21:12:06, bradfitz wrote:
> type blackHoleJar struct{}

If a black hole is the nil case in Client, than
this could be achieved without a blackHoleJar.
See the comment in client.go

[bradfitz, 2011-12-01 21:44:31]

blackHoleJar is 3 lines and saves 4 lines of nil checks.   *shrug*

The API can't ignore secure cookies.  You don't have to implement them this
round, but the API has to permit others to implement them later.
And you definitely shouldn't conflate them all to just a host.

On Thu, Dec 1, 2011 at 1:35 PM, <dr.volker.dobler@gmail.com> wrote:

> Thanks for the comments.  Maybe blackHoleJar
> is not needed at all as there are just two uses
> of the jar in client.go.
>
>
>
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/client.go<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go>
> File src/pkg/net/http/client.go (right):
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/client.go#newcode190<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#newcode190>
> src/pkg/net/http/client.go:**190: var jar CookieJar
> On 2011/12/01 21:12:06, bradfitz wrote:
>
>> jar := c.Jar
>> if jar == nil {
>>     jar = blackHoleJar{}
>> }
>>
>
> If nil means: No cookies during redirects, than
> I would just check this case twice below. I.e.
> here just:
>
> jar := c.Jar
>
> ...
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/client.go#newcode219<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#newcode219>
> src/pkg/net/http/client.go:**219: }
> ... and here
> if jar != nil {
>    for _, cookie := range jar.Select(req.URL) {
>        req.AddCookie(cookie)
>    }
> }
> ...
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/client.go#newcode229<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#newcode229>
> src/pkg/net/http/client.go:**229: jar.Update(r.Cookies(),
> hostname(req.URL))
> ... and here once more:
>
> if jar != nil { jar.Update(r.Cookies(), hostname(req.URL)) }
>
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/jar.go<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go>
> File src/pkg/net/http/jar.go (right):
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/jar.go#newcode23<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newcode23>
> src/pkg/net/http/jar.go:23: Update(cookies []*Cookie, host string)
> On 2011/12/01 21:12:06, bradfitz wrote:
>
>> What about https vs. not?  I think instead of
>> "host string" we might want "url *url.URL" for
>> the URL that sent us the cookies.
>>
>
> I must admit, that I just do not know whether
> it should be allowed to set a Secure cookie from
> a https request and vice versa.  I did not come
> up with any reasons why it should be forbidden.
> That is why I just used host instead of the whole
> URL.
>
>
> http://codereview.appspot.com/**5399043/diff/8002/src/pkg/net/**
>
http/jar.go#newcode33<http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newcode33>
> src/pkg/net/http/jar.go:33: type BlackHoleJar int
> On 2011/12/01 21:12:06, bradfitz wrote:
>
>> type blackHoleJar struct{}
>>
>
> If a black hole is the nil case in Client, than
> this could be achieved without a blackHoleJar.
> See the comment in client.go
>
>
http://codereview.appspot.com/**5399043/<http://codereview.appspot.com/5399043/>
>

[volker.dobler, 2011-12-02 08:17:59]

Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[volker.dobler, 2011-12-02 08:18:43]

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:43: // nil, than each GET and HEAD request gets it's
own
On 2011/12/01 21:12:06, bradfitz wrote:
> its, not it's
> 
> But I think we want the nil case to be a BlackHoleJar.  And I'm not sure I'd
> expose BlackHoleJar publicly.  I'd just say "nil means cookies aren't
> supported".

Done.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/client.go#ne...
src/pkg/net/http/client.go:190: var jar CookieJar
On 2011/12/01 21:12:06, bradfitz wrote:
> jar := c.Jar
> if jar == nil {
>     jar = blackHoleJar{}
> }

Done.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:23: Update(cookies []*Cookie, host string)
On 2011/12/01 21:12:06, bradfitz wrote:
> What about https vs. not?  I think instead of "host string" we might want "url
> *url.URL" for the URL that sent us the cookies.
> 
> And I'd put that argument first.
> 
> And I'd name the methods something like:
> 
> StoreCookies
> GetCookies
> 
> But I'm open to alternate proposals.  (Russ?)

Done.

Replaced host by URL because than Store and Get 
are more symetric, gives mire concise API.

Correct me if I am wrong, but I think, that
handling of secure (aka https-only) cookies should
be done in GetCookies and not in StoreCookies:
You don't want to send a Secore cookie over an
unencrypted wire, but accepting a secure cookie
from http seem legit (to me).

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:33: type BlackHoleJar int
On 2011/12/01 21:12:06, bradfitz wrote:
> type blackHoleJar struct{}

Done.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:33: type BlackHoleJar int
On 2011/12/01 21:12:06, bradfitz wrote:
> type blackHoleJar struct{}

Done.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:35: func (bh BlackHoleJar) Update(cookies []*Cookie,
host string) {}
On 2011/12/01 21:12:06, bradfitz wrote:
> Don't need receiver names here:
> 
> func (blackHoleJar) Foo() ...
> 

Done.

http://codereview.appspot.com/5399043/diff/8002/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:206: type TrivialJar struct {
On 2011/12/01 21:12:06, bradfitz wrote:
> Can we drop this implementation from this CL for now and focus just on the
> Client and interface API changes?
> 
> We can add this back in a subsequent CL, perhaps named differently too
> (MemoryJar? who knows).  Trivial things become less trivial over time.  This
is
> really about storing them in memory, I believe.  Also, the cookie policy logic
> should be separate from the cookie storing/loading policy.  In this
> implementation they're kinda combined.
> 
> The implementations will need tests too.

Done.

Maybe some of the (currently internal) functions above 
should be exported, so that a implementation of CookieJar
could benefit from them.

[bradfitz, 2011-12-02 16:28:44]

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/client.go#n...
src/pkg/net/http/client.go:42: // A nil Jar means cookies are not supported.
// Jar specifies the cookie jar.
// If Jar is nil, cookies are not supported.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/client.go#n...
src/pkg/net/http/client.go:223: jar.StoreCookies(req.URL, r.Cookies())
Let's not call the interface if there are no cookies.  I don't want people
misusing this as some sort of logging hook.

if c := r.Cookies(); len(c) > 0 {
    jar.StoreCookies(req.URL, c)
}

I know I asked for the nil check to be removed so this could be prettier, but
mostly I wanted to remove nil checks.  I don't mind the if.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:22: // Invalid cookies (e.g. cookies with a wildcard top
level domain or
This last sentence (Invalid cookies...) can be removed.  It describes an
implementation, not the interface.

Unless you mean "they SHOULD be ignored".  But I think the comment would be
better on an implementation.

I think this whole comment could be:

// StoreCookies adds the provided cookies to the jar.
// The cookies were sent by the server after requesting URL u.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:31: // The returned slice may be empty or nil.
I think this sentence can go.  It's pretty obvious.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:42: // normalizeCookie returns a copy of cookie where
the domain, path and expires
remove this from this CL.  it's not yet used.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:146: // qualifyCookie checks if a cookie qualifies to be
sent to the url.
remove this too, and everything else in this CL that's not yet used.

let's do this incrementally.

[volker.dobler, 2011-12-02 18:27:32]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[volker.dobler, 2011-12-02 18:28:26]

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/client.go#n...
src/pkg/net/http/client.go:42: // A nil Jar means cookies are not supported.
On 2011/12/02 16:28:44, bradfitz wrote:
> // Jar specifies the cookie jar.
> // If Jar is nil, cookies are not supported.

Done.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/client.go#n...
src/pkg/net/http/client.go:223: jar.StoreCookies(req.URL, r.Cookies())
On 2011/12/02 16:28:44, bradfitz wrote:
> Let's not call the interface if there are no cookies.  I don't want people
> misusing this as some sort of logging hook.
> 
> if c := r.Cookies(); len(c) > 0 {
>     jar.StoreCookies(req.URL, c)
> }
> 
> I know I asked for the nil check to be removed so this could be prettier, but
> mostly I wanted to remove nil checks.  I don't mind the if.

Done.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:22: // Invalid cookies (e.g. cookies with a wildcard top
level domain or
On 2011/12/02 16:28:44, bradfitz wrote:
> This last sentence (Invalid cookies...) can be removed.  It describes an
> implementation, not the interface.
> 
> Unless you mean "they SHOULD be ignored".  But I think the comment would be
> better on an implementation.
> 
> I think this whole comment could be:
> 
> // StoreCookies adds the provided cookies to the jar.
> // The cookies were sent by the server after requesting URL u.

Done.

http://codereview.appspot.com/5399043/diff/10003/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:31: // The returned slice may be empty or nil.
On 2011/12/02 16:28:44, bradfitz wrote:
> I think this sentence can go.  It's pretty obvious.

Done.

[bradfitz, 2011-12-14 00:43:05]

Sorry, I'd forgotten that this wasn't in yet!

Two tiny comments, but nothing real.

After that, LGTM.

Russ, care to look?

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go#newcode1
src/pkg/net/http/jar.go:1: // Copyright 2009 The Go Authors. All rights
reserved.
fix copyright date to 2011

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:26: // blackHoleJar implements a black hole cookie jar:
Whatever you stuck in
s/stuck/stick/

But you could also delete these comments since it's pretty obvious from the
implementation and name and this isn't an exported type.

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:31: func (blackHoleJar) GetCookies(u *url.URL) []*Cookie
           { return nil }
wow, does this pass gofmt, with all that space before the opening brace? 
whatever gofmt says is right, in any case.

[volker.dobler, 2011-12-14 08:35:46]

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go#newcode1
src/pkg/net/http/jar.go:1: // Copyright 2009 The Go Authors. All rights
reserved.
On 2011/12/14 00:43:05, bradfitz wrote:
> fix copyright date to 2011

Done.

http://codereview.appspot.com/5399043/diff/5007/src/pkg/net/http/jar.go#newco...
src/pkg/net/http/jar.go:26: // blackHoleJar implements a black hole cookie jar:
Whatever you stuck in
On 2011/12/14 00:43:05, bradfitz wrote:
> s/stuck/stick/
> 
> But you could also delete these comments since it's pretty obvious from the
> implementation and name and this isn't an exported type.

Done.

[volker.dobler, 2011-12-14 08:36:02]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-12-14 21:22:27]

very nice.

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/client.go#n...
src/pkg/net/http/client.go:43: // If Jar is nil, cookies are not supported.
are not sent in requests and ignored in replies.

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:11: // CookieJar is the interface for a cookie jar.
This doesn't actually say anything if you don't know the jar-gon.

// A CookieJar manages storage and use of cookies in HTTP requests.

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:13: // Implementations of CookieJar must be safe for use
by concurrent goroutines.
s/use by concurrent/concurrent use by multiple/

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:15: // StoreCookies adds the provided cookies to the
jar.
I'd like to avoid the Get in the one method,
and shortening it to Cookies suggests using the
standard HTTP header names (well, pluralized).

// SetCookies handles the receipt of the cookies in a reply for the given URL.
// It may or may not choose to save the cookies, depending on the
// jar's policy and implementation.
SetCookies(u *url.URL, cookies []*Cookie)

// Cookies returns the cookies to send in a request for the given URL.
// It is up to the implementation to honor the standard cookie use restrictions 
// such as in RFC 2019.
Cookies(u *url.URL) []*Cookie

or whatever the RFC is today.

[volker.dobler, 2011-12-15 21:58:48]

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/client.go
File src/pkg/net/http/client.go (right):

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/client.go#n...
src/pkg/net/http/client.go:43: // If Jar is nil, cookies are not supported.
On 2011/12/14 21:22:27, rsc wrote:
> are not sent in requests and ignored in replies.

Done, but used "responses" to be more consistent.

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go
File src/pkg/net/http/jar.go (right):

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:11: // CookieJar is the interface for a cookie jar.
On 2011/12/14 21:22:27, rsc wrote:
> This doesn't actually say anything if you don't know the jar-gon.
> 
> // A CookieJar manages storage and use of cookies in HTTP requests.

Done.

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:13: // Implementations of CookieJar must be safe for use
by concurrent goroutines.
On 2011/12/14 21:22:27, rsc wrote:
> s/use by concurrent/concurrent use by multiple/

Done.

http://codereview.appspot.com/5399043/diff/15002/src/pkg/net/http/jar.go#newc...
src/pkg/net/http/jar.go:15: // StoreCookies adds the provided cookies to the
jar.
On 2011/12/14 21:22:27, rsc wrote:
> I'd like to avoid the Get in the one method,
> and shortening it to Cookies suggests using the
> standard HTTP header names (well, pluralized).
> 
> // SetCookies handles the receipt of the cookies in a reply for the given URL.
> // It may or may not choose to save the cookies, depending on the
> // jar's policy and implementation.
> SetCookies(u *url.URL, cookies []*Cookie)
> 
> // Cookies returns the cookies to send in a request for the given URL.
> // It is up to the implementation to honor the standard cookie use
restrictions 
> // such as in RFC 2019.
> Cookies(u *url.URL) []*Cookie
> 
> or whatever the RFC is today.

Done. Referenced RFC 6265.

[volker.dobler, 2011-12-15 21:59:19]

Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-12-16 15:35:50]

LGTM

[rsc, 2011-12-16 15:48:43]

*** Submitted as http://code.google.com/p/go/source/detail?r=06e9bc864110 ***

net/http: Added interface for a cookie jar.

Types implementing CookieJar may be used in a Client
to persist cookies.

R=bradfitz, rsc
CC=golang-dev
http://codereview.appspot.com/5399043

Committer: Russ Cox <rsc@golang.org>

[rsc, 2011-12-16 15:48:43]

*** Submitted as http://code.google.com/p/go/source/detail?r=06e9bc864110 ***

net/http: Added interface for a cookie jar.

Types implementing CookieJar may be used in a Client
to persist cookies.

R=bradfitz, rsc
CC=golang-dev
http://codereview.appspot.com/5399043

Committer: Russ Cox <rsc@golang.org>