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

Added support for SameSite cookie flag #1615

Merged
merged 11 commits into from Jan 20, 2020
Merged

Conversation

anio
Copy link
Contributor

@anio anio commented Oct 29, 2018

@codecov
Copy link

codecov bot commented Oct 29, 2018

Codecov Report

Merging #1615 into master will increase coverage by 0.01%.
The diff coverage is 99.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1615      +/-   ##
==========================================
+ Coverage    98.5%   98.51%   +0.01%     
==========================================
  Files          40       41       +1     
  Lines        2267     2287      +20     
==========================================
+ Hits         2233     2253      +20     
  Misses         18       18              
  Partials       16       16
Impacted Files Coverage Δ
render/msgpack.go 100% <ø> (ø) ⬆️
binding/default_validator.go 100% <ø> (ø) ⬆️
binding/msgpack.go 100% <ø> (ø) ⬆️
binding/form.go 100% <ø> (ø) ⬆️
render/render.go 100% <ø> (ø) ⬆️
binding/binding_nomsgpack.go 100% <100%> (ø)
path.go 100% <100%> (ø) ⬆️
auth.go 100% <100%> (ø) ⬆️
context.go 98.66% <100%> (ø) ⬆️
render/json.go 87.5% <100%> (ø) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5302dd3...1798a1f. Read the comment docs.

Copy link
Member

@appleboy appleboy left a comment

Choose a reason for hiding this comment

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

Samesite flag only support on Golang v1.11

@appleboy appleboy added this to the 1.4 milestone Nov 1, 2018
@thinkerou thinkerou modified the milestones: 1.4, 1.x Feb 26, 2019
@JFKingsley
Copy link

Is there any chance that we could see this merged? There's several instances in codebases that I maintain where the ability to use this now that Go 1.11 is out would be massively useful.

@appleboy
Copy link
Member

@anio sameSite mode only support on go 1.11 version.

https://golang.org/pkg/net/http/#Cookie

type Cookie struct {
    Name  string
    Value string

    Path       string    // optional
    Domain     string    // optional
    Expires    time.Time // optional
    RawExpires string    // for reading cookies only

    // MaxAge=0 means no 'Max-Age' attribute specified.
    // MaxAge<0 means delete cookie now, equivalently 'Max-Age: 0'
    // MaxAge>0 means Max-Age attribute present and given in seconds
    MaxAge   int
    Secure   bool
    HttpOnly bool
    SameSite SameSite // Go 1.11
    Raw      string
    Unparsed []string // Raw text of unparsed attribute-value pairs
}

@haywirez
Copy link

haywirez commented Jan 7, 2020

Could this issue get some more attention? Chrome 80 with same-site cookies by default is shipping in less than a month: https://chromiumdash.appspot.com/schedule

@haywirez
Copy link

haywirez commented Jan 7, 2020

An earlier comment in this thread that is a request for changes blocking the merge:

Samesite flag only support on Golang v1.11

The README already states that this project requires Go 1.11+. So I don't really see why this shouldn't be merged 🤔 ?

To install Gin package, you need to install Go and set your Go workspace first.

  1. The first need Go installed (version 1.11+ is required), then you can use the below Go command to install Gin.

@thinkerou
Copy link
Member

@appleboy I have fixed, please review, thanks!

@thinkerou thinkerou merged commit f94406a into gin-gonic:master Jan 20, 2020
ThomasObenaus pushed a commit to ThomasObenaus/gin that referenced this pull request Feb 19, 2020
* Added support for SameSite cookie flag

* fixed tests.

* Update context.go

* Update context_test.go

* Update context_test.go

Co-authored-by: thinkerou <thinkerou@gmail.com>
Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@kailin4u
Copy link

kailin4u commented Mar 23, 2020

do we have any strong reason to break back compatibility?
And according to https://semver.org/ version string design,which is a philosophy accepted by go modules,if there is any incompatibility ,the major version should change.

byebyebruce pushed a commit to byebyebruce/gin that referenced this pull request Mar 25, 2020
* Added support for SameSite cookie flag

* fixed tests.

* Update context.go

* Update context_test.go

* Update context_test.go

Co-authored-by: thinkerou <thinkerou@gmail.com>
Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants