-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Get client IP when using Cloudflare #2723
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2723 +/- ##
=======================================
Coverage 98.69% 98.69%
=======================================
Files 41 41
Lines 2070 2074 +4
=======================================
+ Hits 2043 2047 +4
Misses 15 15
Partials 12 12
Continue to review full report at Codecov.
|
Thinking about it, I wonder if rather than using booleans, it might make more sense to create an enum This way, once the next platform is inevitably added, we don't add yet another boolean. The downside is around what to do with the existing Google App Engine flag. |
Yes, I also like |
I can update to use an enum-like. What should I do for the existing Google App Engine flag? Keep that for legacy and "translate" that into the enum? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Wait before you merge.... I want to update it to use an enum. Before we introduce even more legacy. :) just let me know what I should do with the existing Boolean flag for Google App Engine |
@ItalyPaleAle I merged the pull request at first, please commit other pull request using(refactoring) |
Ok. Will do it over the weekend or sooner. What do you want me to do with the existing Boolean for GAE? I can't remove it because it would be backwards-incompatible. I can keep it as a legacy alias that then is translated to the enum? |
@ItalyPaleAle please read https://github.com/gin-gonic/gin/blob/master/deprecated.go for reference, we should deprecated warning at first and remove. |
@thinkerou I made the change in #2739 hope this works! |
Co-authored-by: thinkerou <thinkerou@gmail.com> (cherry picked from commit 6703dea)
Co-authored-by: thinkerou <thinkerou@gmail.com> (cherry picked from commit 6703dea)
Co-authored-by: thinkerou <thinkerou@gmail.com> (cherry picked from commit 6703dea)
Co-authored-by: thinkerou <thinkerou@gmail.com> (cherry picked from commit 6703dea)
Co-authored-by: thinkerou <thinkerou@gmail.com>
Co-authored-by: thinkerou <thinkerou@gmail.com> (cherry picked from commit 6703dea)
Co-authored-by: thinkerou <thinkerou@gmail.com>
Co-authored-by: thinkerou <thinkerou@gmail.com>
When proxying an app through Cloudflare, the user's real IP is set in the CF-Connecting-IP header. This is more convenient than having to manually maintain an allow-list of trusted proxies.
Reference: https://support.cloudflare.com/hc/en-us/articles/200170986-How-does-Cloudflare-handle-HTTP-Request-headers-