This issue was initially reported by @michaelgranzow-avi on #2296.
@airween made an initial attempt to provide a fixed at #2107; As a
consequence of the pull request review - provided by @victorhora,
@zimmerle, and @michaelgranzow-avi - @airween made a second attempt
at #2297. After reviewing by @martinhsv, @zimmerle, I have absorbed
the essential pieces from @airween patch into this one.
This patch differs from @airween's because @airween's patches were
partially working: Key exclusions with regex weren't covered, same
for anchored variables (e.g. ARGS). During the review, I have
highlighted the importance of having elementary test cases. A simple
test case on ARGS could spot the issue. Since that is an important
fix, I don't want to hold this for one more review cycle; therefore,
I am committing the fix myself.
Thank you all involved in the solution of this very own issue.
Using GEOIP_INDEX_CACHE on some older versions of libGeoIP (e.g. 1.5.0
which is the default version on CentOS 7) leads to "Error reading file"
error while opening completely valid GeoIP.dat:
# cat test.c
#include <stdio.h>
#include "GeoIP.h"
int main(void) {
GeoIP *g;
g = GeoIP_open("/tmp/GeoIP.dat", GEOIP_INDEX_CACHE);
if (g == NULL) {
printf("error!\n");
}
GeoIP_delete(g);
exit(0);
}
# cc -lGeoIP -o test test.c
# ./test
Error reading file /tmp/GeoIP.dat
error!
# sed -i -e 's,GEOIP_INDEX_CACHE,GEOIP_MEMORY_CACHE,' test.c
# cc -lGeoIP -o test test.c
# ./test
# geoiplookup -f /tmp/GeoIP.dat -v 8.8.8.8
GeoIP Country Edition: GEO-106FREE 20180327 Build 1 Copyright (c) 2018 MaxMind Inc All Rights Reserved
Also tested with recent GeoLite databases converted from new format
into legacy format, distributed here:
https://mailfud.org/geoip-legacy/
This commit fixes quite a few odd things in regex code:
* Lack of encapsulation.
* Non-method functions for matching without retrieving all groups.
* Regex class being copyable without proper copy-constructor (potential UAF
and double free due to pointer members m_pc and m_pce).
* Redundant SMatch::m_length, which always equals to match.size() anyway.
* Weird SMatch::size_ member which is initialized only by one of the three matching
functions, and equals to the return value of that function anyways.
* Several places in code having std::string value instead of reference.
When reloading Nginx, there is a race condition which is visible under high
load. As the logging mutex is shared between multiple workers, when a worker
is sent a stop signal during a reload, and the log mutex is held, write()
will never return, which means that the mutex will never unlock. As other
workers share this mutex, they will deadlock.
fcntl does not suffer from this issue.
new_debug_log was unitialized during an error code path.
Fixed this by explicit initializing it to NULL and fixing the order of
the error labels. They now present the correct (reverse) order of the
goto statements.
