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

cookie: add possibility to disable same site attribute #1428

Merged
merged 3 commits into from
Jul 16, 2021
Merged

cookie: add possibility to disable same site attribute #1428

merged 3 commits into from
Jul 16, 2021

Conversation

wja513
Copy link
Contributor

@wja513 wja513 commented Jul 9, 2021

fix some old browsers cann't set the cookie if it contains "SameSite" attribute.

ctx.Cookie(c, &fiber.Cookie{
	Name:     cookie.Name,
	Value:    cookie.Value,
        ...
	//SameSite: "",// this attribute should be constant.(why not use fasthttp constant directly?)
})

@welcome
Copy link

welcome bot commented Jul 9, 2021

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

ReneWerner87 commented Jul 9, 2021

@wja513 please check the tests
image

please add new ones for your customization

ctx.go Outdated Show resolved Hide resolved
@wja513 wja513 requested a review from ReneWerner87 July 14, 2021 01:53
@ReneWerner87 ReneWerner87 changed the title fix some old browsers cann't set the cookie if it contains SameSite p… cookie: add possibility to disable same site attribute Jul 16, 2021
@ReneWerner87 ReneWerner87 merged commit dd45be6 into gofiber:master Jul 16, 2021
@welcome
Copy link

welcome bot commented Jul 16, 2021

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

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.

2 participants