-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Bugfix/1834 Fix X-Real-IP bug #2007
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2007 +/- ##
==========================================
- Coverage 92.21% 92.21% -0.01%
==========================================
Files 37 37
Lines 3019 3018 -1
==========================================
- Hits 2784 2783 -1
Misses 148 148
Partials 87 87
Continue to review full report at Codecov.
|
Going to look into this soon |
Thank you @lammel |
fb12a8f
to
635adb9
Compare
…port older Go versions
@lammel please review. I have reworked all tests to be more "readable", added comments. Fixed couple of headers to be in canonical format. Tests are way longer now but these older tests were so unreadable with their double loops and constants. For readability using text and copying it here and there is better than code that takes serious IDE line-hopping to understand what is actually used as inputs |
Awesome, that was what I meant with my comments, thanks for doing the work I wanted to do @aldas ! |
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.
Looks very good to me.
TrustLinkLocal(false), | ||
TrustPrivateNet(false), | ||
// this is private IPv6 ip | ||
// CIDR Notation: 2001:0db8:0000:0000:0000:0000:0000:0000/48 |
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.
For better readability for different tab sizes better uses spaces for aligning the values here (github also displays with tabsize of 4).
Not really important though.
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.
seems that Echo .editorconfig
sets indent_size = 2
ip_test.go
Outdated
// Address: 8.8.8.8 | ||
// Range start: 8.8.8.0 | ||
// Range end: 8.8.8.255 | ||
givenRange: "8.8.8.8/24", |
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.
Usually a trust subnet would be specified using the base address with subnet mask, in that case 8.8.8.0/24. Unless this was intentional, maybe we should change that.
(also not really important)
This PR Fixes #1834
realIP
value only whenrealIP
is not empty and is a valid and trusted IP.directIP
value.ipForInternalRealIP
value for comparing internal XRealIP test cases.See #1834/comment