-
Notifications
You must be signed in to change notification settings - Fork 3
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 ip algorithm #237
Update ip algorithm #237
Conversation
672f91c
to
d914793
Compare
Codecov Report
@@ Coverage Diff @@
## master #237 +/- ##
==========================================
+ Coverage 63.87% 71.19% +7.32%
==========================================
Files 88 25 -63
Lines 5655 3222 -2433
Branches 1794 716 -1078
==========================================
- Hits 3612 2294 -1318
+ Misses 972 553 -419
+ Partials 1071 375 -696
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 64 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
d914793
to
66f4449
Compare
I will update the trace algorithm when this gets merged @Anilm3 |
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 good although perhaps a few more parsing tests for the new headers would be useful.
tests/extension/extract_ip_addr.phpt
Outdated
test('via', '2001:abcf:1f::55'); | ||
|
||
test('true_client_ip', '8.8.8.8'); | ||
test('fastly_client_ip', '7.7.7.2'); |
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.
Perhaps a few more tests with valid / invalid IPs as wells as IPv4 and IPv6 and ports. For all three new headers...
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.
Fair enough
Description
Update ip detection algorithm to pick the most priority header. Before if there were ips in multiple headers, it would report all of them
Motivation
Additional Notes
Describe how to test your changes
Readiness checklist
Release checklist