kataras / iris

The fastest HTTP/2 Go Web Framework. New, modern and easy to learn. Fast development with Code you control. Unbeatable cost-performance ratio :rocket:
https://www.iris-go.com
BSD 3-Clause "New" or "Revised" License
25.04k stars 2.47k forks source link

There is no option to delete a cookie with a particular path #1018

Closed dibyendu closed 5 years ago

dibyendu commented 6 years ago

Consider the following code snippet from iris/context/context.go

// it takes the current url path to set the cookie with
func (ctx *context) SetCookieKV(name, value string) {
    c := &http.Cookie{}
    c.Name = name
    c.Value = url.QueryEscape(value)
    c.HttpOnly = true
/*
        c.Path = "/abc/xyz"    // for url = http://localhost/abc/xyz
*/
    c.Expires = time.Now().Add(SetCookieKVExpiration)
    c.MaxAge = int(SetCookieKVExpiration.Seconds())
    ctx.SetCookie(c)
}

// But this function doesn't have any parameter to delete a cookie with a particular path.
func (ctx *context) RemoveCookie(name string) {
    c := &http.Cookie{}
    c.Name = name
    c.Value = ""
    c.Path = "/"        // path is hard-coded
    c.HttpOnly = true
    // RFC says 1 second, but let's do it 1 minute to make sure is working
    exp := time.Now().Add(-time.Duration(1) * time.Minute)
    c.Expires = exp
    c.MaxAge = -1
    ctx.SetCookie(c)
    // delete request's cookie also, which is temporary available.
    ctx.request.Header.Set("Cookie", "")
}

Path is hard-coded here: https://github.com/kataras/iris/blob/a7364876e0d1b8bd60acf94f17f6d1341b16c617/context/context.go#L3039

An use case

If we set a cookie like this:

app.PartyFunc("/abc", func(p iris.Party) {
    // http://localhost/abc/xyz
    p.Get("/xyz", func (ctx iris.Context) {
        ctx.SetCookieKV("key", "value")
        // the cookie path is set to "/abc/xyz"

        ctx.JSON(iris.Map{"hello": "world"})
    })
})

the cookie path is set to "/abc/xyz". Now if we try to remove the cookie by ctx.RemoveCookie("key") from another handler for a different url (like http://localhost/pqr), it doesn't delete the cookie.

Proposal

There should be an optional parameter path in this function: func (ctx *context) RemoveCookie(name string) that overrides the hard-coded value "/".

https://github.com/kataras/iris/blob/a7364876e0d1b8bd60acf94f17f6d1341b16c617/context/context.go#L3035

P.S: This is how it is implemented in other popular frameworks (e.g: python webapp2)

https://github.com/moraes/webapp-improved/blob/0e6218dcd3ba2e0ba0c6a6c87ba4fbe1eab287c4/lib/WebOb-1.0.8/webob/response.py#L635

kataras commented 6 years ago

Hello @dibyendu, the title is wrong, you can always do whatever you like with Iris, because Iris is 100% compatible with the net/http package, so you can still use your existing net/http knowedge even if one of Iris' methods doesn't suit your needs. Although Iris has the ctx.SetCookie which gives you the way to set a custom http.Cookie, I know we have a lot of methods but that's why we have an interface, to be easy to catch up. Therefore you can also delete/set a cookie for a specific path using:

c := &http.Cookie{
    Name:     "name",
    Value:    "",
    Path:     "/your_path",
    Expires: time.Unix(0, 0),
    HttpOnly: true,
}

ctx.SetCookie(c) // or even http.SetCookie(ctx.ResponseWriter(), ctx.Request())

Go is not Python, If I accept your proposal the context's RemoveCookie will look like (name string, path ...string) and I don't think that's a proper design because it allows you to change only the path, what about others? If someone else will ask for Domain as well we will have to make a breaking change (in Go, the variadic parameters should be always last). Instead we can do something else which will not break the API in any circumstances.

type CookieOption func(*http.Cookie)
func (ctx *context) RemoveCookie(name string, options ...CookieOption)
// usage:

ctx.RemoveCookie("name", func(c *http.Cookie) {
  c.Path = "/my/custom/path
})
// or we can take it even further:

func CookiePath(path string) CookieOption {
  return func(c *http.Cookie) {
     c.Path = path
  }
}

func CookieHTTPOnly(c *http.Cookie) {
     c.HttpOnly = true
}

// usage:

ctx.RemoveCookie("name", CookiePath("my/custom/path"), CookieHTTPOnly)

Which will use the options to modify the c := &http.Cookie{} right before sent to the response. I'll implement that in a minute, stay tuned by starring the Iris github repository, you see that you have great opportunity and potential to learn how to be better programmer just by using Iris and reading its github issues, it's free knowledge so grab it :)

kataras commented 6 years ago

Also,

When we don't set the cookie's path, cookie header value will have its path equal to the current path, that's general knowledge.

On the SetCookieKV we don't set the path, so cookie added to the current path only. But on RemoveCookie we do cookie.Path = "/", this is the only important here. It was time to change it, so the SetCookieKV will be changed in order to set the cookie.Path = "/" as well [ default to "/" instead of the current path because end-developer will expect the cookie to be used on any path ], user will be able to change it to another path or to the current one by setting it to empty: iris.CookiePath("") or iris.CookieCleanPath. Both of those(SetCookieKV and RemoveCookie) will accept the variadic CookieOption.

Almost finished, I'm writing the cookie examples for your learning curve (although cookies are used at a lot of examples already).

kataras commented 6 years ago

Here you're, read the code and the comments, as always iris is 100% code documented: https://github.com/kataras/iris/commit/574414a64ed3d8736c836d476e6304d915f4a511

Example: https://github.com/kataras/iris/blob/master/_examples/cookies/basic/main.go

Upgrade with: go get -u github.com/kataras/iris

Have fun and thank you, with that simple post and the help you needed for that you fed me with even more ideas that will be applied soon, more features to help you on cookie managment are coming as well, stay tuned!

dibyendu commented 6 years ago

Oh, that was quite a fast update :exclamation: :open_mouth: I wasn't expecting this to be implemented so soon. Thank you very much for this quick feedback. The generic CookieOption seems to be a perfect solution for this. :+1:

kataras commented 6 years ago

Yeah!! We make the difference here, and the smallest thing you can do to help Iris too is by starring it. In this repo you will always be heard even if that results to spending my own personal free time to accomplish something that you need today in order to complete your job or your personal project when using Iris, not after a year like other authors do(!). Thank you again for being an active member, don't hesisate to ask or propose more things in the future!

Join to: https://chat.iris-go.com Please complete the user experience form when ever you feel free to: https://goo.gl/forms/lnRbVgA6ICTkPyk02