net/http: drop spaces in SetCookie values

[gopherbot]

by frel8817:

What steps will reproduce the problem?

1. Set a cookie in a http-handler.
 
expiration := time.Now()
expiration.AddDate(1, 0, 0)
cookie := http.Cookie{Name: "browserID", Value: "my value", Expires:
expiration}
http.SetCookie(w, &cookie) 

2. Read the cookie.

cookie, err := r.Cookie("browserID")
if err != nil {
    fmt.Fprintf(w, err.Error())
    return
}
fmt.Fprintf(w, cookie.Name) 



What is the expected output?

browserID



What do you see instead?

nothing



Which compiler are you using (5g, 6g, 8g, gccgo)?

go_appengine_sdk_linux_amd64-go1beta2



Which operating system are you using?

Linux Mint 11 x64



Please provide any additional information below.

See discussion,
https://groups.google.com/forum/?fromgroups#!topic/golang-nuts/UNzkKcQxC9Y

[bradfitz]

Comment 1:

Your expiration value is wrong.
AddDate doesn't mutate its receiver; it returns a new time.

Status changed to Invalid.

[gopherbot]

Comment 2 by frel8817:

Sorry, my mistake. Replace first two lines with:
expiration := time.Now().AddDate(1, 0, 0)

[rsc]

Comment 3:

Don't use cookies with spaces in them.
Your example works fine if you use "myvalue" instead of "my value".
The Cookie parser rejects cookie values with spaces in them.
This might or might not be a mistake
SetCookie blindly sends cookies with spaces in them,
which also might or might not be a mistake.
This can probably wait until after Go 1.
Full test program:
package main
import (
    "fmt"
    "net/http"
    "log"
    "time"
)
func main() {
    http.HandleFunc("/", setcookie)
    http.HandleFunc("/readcookie", readcookie)
    log.Fatal(http.ListenAndServe("127.0.0.1:8080", nil))
}
func setcookie(w http.ResponseWriter, req *http.Request) {
    expiration := time.Now().AddDate(1, 0, 0)
    cookie := http.Cookie{Name: "foo", Value: "myvalue", Expires: expiration}
    http.SetCookie(w, &cookie)
    http.Redirect(w, req, "/readcookie", 307)
}
func readcookie(w http.ResponseWriter, req *http.Request) {
    c, err := req.Cookie("foo")
    fmt.Fprintf(w, "foo: %v %v\n", c, err)
    fmt.Fprintf(w, "all:\n")
    for _, c := range req.Cookies() {
        fmt.Fprintf(w, "\t%s\n", c)
    }
}

Labels changed: added priority-later, removed priority-triage.

Owner changed to builder@golang.org.

Status changed to Accepted.

[rsc]

Comment 4:

Labels changed: added go1.1maybe.

[vdobler]

Comment 5:

According to RFC 6265 spaces are not allowed in the
value of a cookie (not even if double-quoted). 
See http://tools.ietf.org/html/rfc6265#section-4.1.1
Unfortunately RFC 2109 did allow spaces in
double quoted values. 
RFC 6265 is pretty clear that the cookie-value is
not a string but just some octets.  Section 4.1.1
states that values should be encoded and suggests
Base64. See also note at end of section 5.4.
As SetCookie does not return an error its possibilities
on an invalid cookie value are limited:
 - set the given (invalid) value (current behaviour)
 - ignore the cookie
 - panic 
 - encode the value
As it is called SetCookie and not MustSetCookie panic is
not an option.  Just ignoring it without any feedback
is unpolite.  Encoding the value is a hairy issue as
there is no single right way to do it.
Go typically is very explicit, so an implicit, automatic
encoding in SetCookie if the value contains illegal 
characters feels strange. Additionally it adds the 
overhead of deciding whether to encode or not to any
call.
I'd suggest a better documentation of SetCookie with 
an example of how to manually encode the cookie value
beforehand.

[rsc]

Comment 6:

Issue #4613 has been merged into this issue.

[rsc]

Comment 7:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

[rsc]

Comment 8:

Of the various bad choices, eliding spaces seems best.

Labels changed: added go1.2.

[rsc]

Comment 9:

Labels changed: added feature.

[bradfitz]

Comment 10:

Was logging + ignoring considered? That's not without precedent in the http package.

[rsc]

Comment 11:

Can you point to one? I looked and found some log + die but no log + ignore.

[bradfitz]

Comment 12:

func (w *response) WriteHeader(code int) {
    if w.conn.hijacked() {
                log.Print("http: response.WriteHeader on hijacked connection")
                return
        }
        if w.wroteHeader {
                log.Print("http: multiple response.WriteHeader calls")
                return
        }

[rsc]

Comment 13:

Dunno how I missed those. Okay log.Print + drop (ignore) spaces, then?
I don't think we should omit the cookie entirely: I think it will be easier
for people to debug if we're sending _something_. Rewriting spaces to
underscores is fine too. I thought about rewriting to ␣ but I bet UTF-8 is
not allowed in cookie values.

[bradfitz]

Comment 14:

https://golang.org/cl/12204043

Status changed to Started.

[bradfitz]

Comment 15:

This issue was closed by revision 17d803d.

Status changed to Fixed.