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

✨ update: tls.ClientHelloInfo in Ctx #2011

Merged
merged 14 commits into from Aug 19, 2022
Merged

Conversation

thomasdseao
Copy link
Contributor

This PR is related to a request : https://github.com/orgs/gofiber/projects/1/views/4

Add the function ClientHelloInfo to the fiber Ctx so everybody can access it from handler.

@welcome
Copy link

welcome bot commented Aug 12, 2022

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

app.go Outdated Show resolved Hide resolved
ctx_test.go Outdated Show resolved Hide resolved
@efectn efectn added this to the v3 milestone Aug 12, 2022
@efectn efectn linked an issue Aug 12, 2022 that may be closed by this pull request
@ReneWerner87 ReneWerner87 removed the v3 label Aug 15, 2022
@ReneWerner87 ReneWerner87 changed the title ✨ v3 (update): tls.ClientHelloInfo in Ctx ✨(update): tls.ClientHelloInfo in Ctx Aug 15, 2022
@efectn
Copy link
Member

efectn commented Aug 15, 2022

I think this method should be in App. Can you move it into the app?

@efectn efectn changed the title ✨(update): tls.ClientHelloInfo in Ctx ✨update: tls.ClientHelloInfo in Ctx Aug 15, 2022
@ja3abuser
Copy link

I think this method should be in App. Can you move it into the app?

@efectn Why? I think it must be in context directly, not in App()

@efectn
Copy link
Member

efectn commented Aug 15, 2022

I think this method should be in App. Can you move it into the app?

@efectn Why? I think it must be in context directly, not in App()

It doesn't have context-specific thing

@ja3abuser
Copy link

I think this method should be in App. Can you move it into the app?

@efectn Why? I think it must be in context directly, not in App()

It doesn't have context-specific thing

tls.ClientHelloInfo, is sent by the browser when the user connects to the website, so I think it should be in context.

@thomasdseao
Copy link
Contributor Author

Yes the CHI is sent by the client, that's why I put it in the context.
But if you think this is more compliant with Fiber logical to have it in App, I can move it.

@ReneWerner87
Copy link
Member

@thomasdseao
can you please add a pull request to our documentation repository so that everyone knows what the point of it is and how to use it?
https://github.com/gofiber/docs/pulls

@thomasdseao
Copy link
Contributor Author

@ReneWerner87 That's done : gofiber/docs#271

ctx.go Outdated Show resolved Hide resolved
@efectn efectn changed the title ✨update: tls.ClientHelloInfo in Ctx ✨ update: tls.ClientHelloInfo in Ctx Aug 18, 2022
@ReneWerner87 ReneWerner87 merged commit 2edcf95 into gofiber:master Aug 19, 2022
@welcome
Copy link

welcome bot commented Aug 19, 2022

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.

🚀 v3 Request: tls.ClientHelloInfo in handlers
4 participants