Skip to content

Commit 97762dc

Browse files
zimmerleFelipe Zimmerle
authored and
Felipe Zimmerle
committed
Avoids to cleanup GeoIp on ModSecurity destructor
GeoIp is already being cleaned elsewhere. Fix #2041
1 parent ab67547 commit 97762dc

File tree

5 files changed

+19
-13
lines changed

5 files changed

+19
-13
lines changed

CHANGES

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
v3.x.y - YYYY-MMM-DD (to be released)
22
-------------------------------------
33

4+
- Avoids to cleanup GeoIp on ModSecurity destructor
5+
[#2041 - @zimmerle, @jptosso, @victorhora]
46
- Fix memory leak of RuleMessages objects
57
[#2376 - @WGH-, @martinhsv]
68
- Produce not-supported error for ctl:forceRequestBodyVariable

src/modsecurity.cc

-3
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,6 @@ ModSecurity::~ModSecurity() {
9595
#ifdef MSC_WITH_CURL
9696
curl_global_cleanup();
9797
#endif
98-
#ifdef WITH_GEOIP
99-
Utils::GeoLookup::getInstance().cleanUp();
100-
#endif
10198
#ifdef WITH_LIBXML2
10299
xmlCleanupParser();
103100
#endif

src/utils/geo_lookup.cc

+10-6
Original file line numberDiff line numberDiff line change
@@ -63,22 +63,26 @@ bool GeoLookup::setDataBase(const std::string& filePath,
6363
std::string intGeo;
6464
#endif
6565

66+
if (m_version != NOT_LOADED) {
67+
cleanUp();
68+
}
69+
6670
#ifdef WITH_MAXMIND
6771
int status = MMDB_open(filePath.c_str(), MMDB_MODE_MMAP, &mmdb);
68-
if (status != MMDB_SUCCESS) {
69-
intMax.assign("libMaxMind: Can't open: " + std::string(MMDB_strerror(status)) + ".");
70-
} else {
72+
if (status == MMDB_SUCCESS) {
7173
m_version = VERSION_MAXMIND;
74+
} else {
75+
intMax.assign("libMaxMind: Can't open: " + std::string(MMDB_strerror(status)) + ".");
7276
}
7377
#endif
7478

7579
#ifdef WITH_GEOIP
7680
if (m_version == NOT_LOADED) {
7781
m_gi = GeoIP_open(filePath.c_str(), GEOIP_MEMORY_CACHE);
78-
if (m_gi == NULL) {
79-
intGeo.append("GeoIP: Can't open: " + filePath + ".");
80-
} else {
82+
if (m_gi) {
8183
m_version = VERSION_GEOIP;
84+
} else {
85+
intGeo.append("GeoIP: Can't open: " + filePath + ".");
8286
}
8387
}
8488
#endif

src/utils/geo_lookup.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,16 @@ class GeoLookup {
5555
private:
5656
GeoLookup() :
5757
m_version(NOT_LOADED)
58+
#if WITH_MAXMIND
59+
,mmdb()
60+
#endif
5861
#if WITH_GEOIP
5962
,m_gi(NULL)
6063
#endif
6164
{ }
6265
~GeoLookup();
63-
GeoLookup(GeoLookup const&);
64-
void operator=(GeoLookup const&);
66+
GeoLookup(GeoLookup const&) = delete;
67+
void operator=(GeoLookup const&) = delete;
6568

6669
GeoLookupVersion m_version;
6770
#if WITH_MAXMIND

test/cppcheck_suppressions.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,14 @@ functionStatic:src/engine/lua.h:80
5151
functionConst:src/utils/geo_lookup.h:49
5252
useInitializationList:src/operators/rbl.h:69
5353
constStatement:test/common/modsecurity_test.cc:82
54-
danglingTemporaryLifetime:src/modsecurity.cc:204
54+
danglingTemporaryLifetime:src/modsecurity.cc:203
55+
danglingTempReference:src/modsecurity.cc:203
5556
functionStatic:src/operators/geo_lookup.h:35
5657
duplicateBreak:src/operators/validate_utf8_encoding.cc
5758
duplicateBranch:src/request_body_processor/multipart.cc:91
5859
syntaxError:src/transaction.cc:62
5960
noConstructor:src/variables/variable.h:152
6061
duplicateBranch:src/request_body_processor/multipart.cc:93
61-
danglingTempReference:src/modsecurity.cc:204
6262
knownConditionTrueFalse:src/operators/validate_url_encoding.cc:79
6363
knownConditionTrueFalse:src/operators/verify_svnr.cc:90
6464
noConstructor:src/actions/rule_id.h:30

0 commit comments

Comments
 (0)