Skip to content
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

Create functional option for ctx.SetCookie #208

Merged
merged 8 commits into from
Nov 13, 2020

Conversation

zeripath
Copy link
Contributor

Allowing func(*http.Cookie) to be passed into the SetCookie call will allow us to extend SetCookie to set the SameSite attribute easily.

(This PR also does some slight cleanup to context.go fixing minor lint issues.)

Signed-off-by: Andrew Thornton <art27@cantab.net>
…the cookie

Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
@6543
Copy link
Contributor

6543 commented Nov 11, 2020

@unknwon can you have a look at it?

it also would cover #206 ... but in a more general way :)

Copy link
Contributor

@unknwon unknwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Can we also have some tests to cover uses of these helpers?

Likely will be added to here:

macaron/context_test.go

Lines 206 to 212 in d229aed

Convey("Set and get cookie", func() {
m.Get("/set", func(ctx *Context) {
t, err := time.Parse(time.RFC1123, "Sun, 13 Mar 2016 01:29:26 UTC")
So(err, ShouldBeNil)
ctx.SetCookie("user", "Unknwon", 1, "/", "localhost", true, true, t)
ctx.SetCookie("user", "Unknwon", int32(1), "/", "localhost", 1)
ctx.SetCookie("user", "Unknwon", int64(1))

context.go Outdated Show resolved Hide resolved
context.go Outdated Show resolved Hide resolved
cookie/helper.go Outdated Show resolved Hide resolved
cookie/helper.go Outdated Show resolved Hide resolved
zeripath and others added 3 commits November 12, 2020 11:58
Co-authored-by: ᴜɴᴋɴᴡᴏɴ <u@gogs.io>
Signed-off-by: Andrew Thornton <art27@cantab.net>
context_test.go Outdated Show resolved Hide resolved
Co-authored-by: 6543 <6543@obermui.de>
@6543
Copy link
Contributor

6543 commented Nov 12, 2020

@unknwon It's ready again :)

Copy link
Contributor

@unknwon unknwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow up!

@unknwon unknwon merged commit 6f0734a into go-macaron:master Nov 13, 2020
@unknwon
Copy link
Contributor

unknwon commented Nov 13, 2020

https://github.com/go-macaron/macaron/releases/tag/v1.4.0 has been published for this merge.

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

Successfully merging this pull request may close these issues.

3 participants