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

Fix checking whether source address in X-Forwarded-For is ipv4 or ipv6 #72

Merged
merged 5 commits into from
Feb 7, 2024

Conversation

hparzych
Copy link
Contributor

No description provided.

@hparzych hparzych requested review from przsus and jakubrak January 29, 2024 06:49
@hparzych hparzych force-pushed the fix/handling-mismatch-x-forwarded-for-src-addr branch 2 times, most recently from f88b73d to b5092fd Compare January 29, 2024 06:54
przsus
przsus previously approved these changes Jan 29, 2024
@hparzych hparzych force-pushed the fix/handling-mismatch-x-forwarded-for-src-addr branch from b5092fd to 09df500 Compare January 29, 2024 07:20
@hparzych hparzych force-pushed the fix/handling-mismatch-x-forwarded-for-src-addr branch from 09df500 to 368bb5f Compare January 29, 2024 08:48
@dynatrace-cla-bot
Copy link
Member

dynatrace-cla-bot commented Feb 1, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@przsus przsus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}
return ipChecker.isV6AddressExternal(clientAddrBinary);
}

static bool isClientExternal(const IpAddressChecker& ipChecker, const std::string& addr, bool isIpV6) {
return isIpV6 ? isIpv6ClientExternal(ipChecker, addr) : isIpv4ClientExternal(ipChecker, addr);
static bool isClientExternal(const IpAddressChecker& ipChecker, const std::string& addr, bool isIpv6) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static bool isClientExternal(const IpAddressChecker& ipChecker, const std::string& addr, bool isIpv6) {
static bool isClientExternal(const IpAddressChecker& ipChecker, const std::string& addr, bool isPossiblyIpv6) {

}

static bool isClientExternal(const IpAddressChecker& ipChecker, const std::string& addr) {
bool isPossiblyIpv6{std::count(addr.begin(), addr.end(), ':') >= 2};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that Ipv6-checking logic should not be hidden inside a isClientExternal wrapper and should rather be called outside isClientExternal.

@hparzych hparzych merged commit e7e93a8 into main Feb 7, 2024
3 checks passed
@hparzych hparzych deleted the fix/handling-mismatch-x-forwarded-for-src-addr branch February 7, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants