Validate cookie name #278
Validate cookie name #278
Conversation
…sCookieNameValid check
|
Not a bad idea. Unrelated quick tip: your links are broken. It's because you used the "Add a link" button in the github comment editor, and pasted the url into the title part (between |
|
@ploxiln thanks for spotting that. Should have double checked... |
| func validateCookieName(o *Options, msgs []string) []string { | ||
| cookie := &http.Cookie{Name: o.CookieName} | ||
| if cookie.String() == "" { | ||
| return append(msgs, "invalid cookie name: "+o.CookieName) |
There was a problem hiding this comment.
Can you use fmt.Sprintf verb %q here?
There was a problem hiding this comment.
Yeah sure... Should the usage me made a bit more consistent? I.e func parseSignatureKey uses concatenation, whereas others use fmt.Sprintf?
There was a problem hiding this comment.
Let's leave this PR scoped to this message.
concatenation is "OK" (it's not going to panic or anything) it's just that when the purpose of an error is to highlight unusual values (like a null byte, or a \r) %q can greatly clarify things vs %s.
|
@tanuck thanks for your contribution. I just had one small comment before we merge this |
This fixes #264
(http.Cookie).String()will silently return an empty string when the cookie name contains a rune not in the following table https://github.com/golang/net/blob/master/lex/httplex/httplex.go#L17With this patch I've added an additional validator to check the cookie name at boot. I've had to 'mock' a
http.Cookieas the http package doesn't export the check function, here: https://github.com/golang/go/blob/master/src/net/http/cookie.go#L368Regards,
James