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

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

Open
dibyendu opened this Issue Jun 1, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@dibyendu
Contributor

dibyendu commented Jun 1, 2018

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:

c.Path = "/"

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 "/".

func (ctx *context) RemoveCookie(name string) {

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

This comment has been minimized.

Owner

kataras commented Jun 2, 2018

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

This comment has been minimized.

Owner

kataras commented Jun 2, 2018

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 added a commit that referenced this issue Jun 2, 2018

Some minor but helpful additions, like `CookieOption`. Relative: #1018.…
… Simple cookies example added too. Cookie encoding (side by side with the already session's cookie id encoding) and version upgrade will come tomorrow with a new HISTORY.md entry as well, stay tuned!
@kataras

This comment has been minimized.

Owner

kataras commented Jun 2, 2018

Here you're, read the code and the comments, as always iris is 100% code documented: 574414a

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!

kataras added a commit that referenced this issue Jun 2, 2018

Cookies: Ability to set custom cookie encoders to encode the cookie's…
… value before sent by `ctx.SetCookie` and `ctx.SetCookieKV` and cookie decoders to decode the cookie's value when retrieving from `ctx.GetCookie`. That was the second and final part relative to a community's question at: #1018
@dibyendu

This comment has been minimized.

Contributor

dibyendu commented Jun 2, 2018

Oh, that was quite a fast update ❗️ 😮
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. 👍

@kataras

This comment has been minimized.

Owner

kataras commented Jun 2, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment