Bug 4828: Use feature detection for IPFilter API/ABI checks#177
Bug 4828: Use feature detection for IPFilter API/ABI checks#177yadij merged 3 commits intosquid-cache:masterfrom
Conversation
Solaris 10+ backported IPFiter v5 features to their v4.1.9 which breaks the IPFilterv4 logic when IPv6 is received. Resulting in crashes. see bug 4828
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
Please see inlined comments for one potentially important concern before committing this.
I am approving this PR without understanding the code just to avoid blocking this fix. I assume it is well-tested and well-understood by you and/or the folks you trust enough to commit this.
src/ip/Intercept.cc
Outdated
| #if HAVE_NATLOOKUP_NL_INIPADDR_IN6 | ||
| // warn once every 10 at critical level, then push down a level each repeated event | ||
| static int warningLevel = DBG_CRITICAL; | ||
| debugs(89, warningLevel, "IPF (IPFilter v4) NAT does not support IPv6. Please upgrade to IPFilter v5.1"); |
There was a problem hiding this comment.
Since we are no longer relying on the IPFilter version for this check, should we replace "IPF (IPFilter v4)" with something like "Your IPF" or "Your IPFilter"?
| // for NAT lookup set local and remote IP:port's | ||
| if (newConn->remote.isIPv6()) { | ||
| #if IPFILTER_VERSION < 5000003 | ||
| #if HAVE_NATLOOKUP_NL_INIPADDR_IN6 |
There was a problem hiding this comment.
I know nothing about these macros, but please double check this condition -- it looks like it is reversed:
#if HAVE_FOO
complain that we do not have something we need
#else
// OK, so we do not have FOO, but we use foo in an assignment?!
newConn->local.getInAddr(natLookup.nl_inipaddr.in6);
#endif
squid-anubis
added
the
M-waiting-staging-checks
Solaris 10+ backported IPFiter v5 features to their v4.1.9 which breaks the IPFilterv4 logic when IPv6 is received. Resulting in crashes when IPFilver version detection builds for IPv4-only code. Fix by switching to feature detection of the relevant library symbols instead of library version.
squid-anubis
added
M-passed-staging-checks
squid-anubis
added
M-failed-other
…che#177) Solaris 10+ backported IPFiter v5 features to their v4.1.9 which breaks the IPFilterv4 logic when IPv6 is received. Resulting in crashes. see bug 4828
Solaris 10+ backported IPFiter v5 features to their v4.1.9 which breaks the IPFilterv4 logic when IPv6 is received. Resulting in crashes. see bug 4828
|
This is broken in quite a few ways. For instance, trying to compile on NetBSD highlights:
Patch for autoconf is here: Patch for ip/Intercept.cc is here: |
|
@sborrill, thank you for letting us know! Unfortunately, a bug report filed as a comment in an old closed PR may be mishandled. If you can, please submit a pull request fixing this regression. |
rousskov
removed
the
M-failed-other
4 participants
Solaris 10+ backported IPFiter v5 features to their v4.1.9 which breaks
the IPFilterv4 logic when IPv6 is received. Resulting in crashes when
IPFilver version detection builds for IPv4-only code.
Fix by switching to feature detection of the relevant library symbols
instead of library version.