- Some versions of gcc/libc require setting the pthread flag when using
std::thread, which to implement it.
- This was found compiling the library in a Debian (bullseye) container.
- Test results output escape characters to highlight whether the test
passed or failed. Additionally, the input & output for each test can
include non-ASCII characters. These characters break parsing of
results (.log & .trs files) with grep, as the files are interpreted
to be binary.
- This is controlled by specifying the 'mtstress' argument when running
`unit_test`.
- The goal is to detect if the operator/transformation fails in this
context.
- In this mode, the test will be executed 5'000 times in 50 threads
concurrently.
- Allocation & initialization of the operator/transformation is
performed once in the main thread, while the evaluation is executed in
the threads.
- This is consistent with the library's support for multithreading,
where initialization and loading of rules is expected to run once.
See issue #3215.
- Leverage std::size to determine buffer size at compile time.
- Simplified 'TimeMon::evaluate' implementation as it was using strftime
to get the month, convert the string to int, and then decrement it by
one to make it zero based. This same value is already available in
the 'struct tm' previously generated with the call to localtime_r (and
where the month is already zero-based)
- std::ctime returns a pointer to a string that "may be shared between
std::asctime and std::ctime, and may be overwritten on each invocation
of any of those functions.".
- https://en.cppreference.com/w/cpp/chrono/c/ctime
- Replaced with call to strftime to generate the same string
representation (using the format string: %c)
- Leveraged localtime_r (which is thread-safe) to convert time_t to
struct tm, as required by strftime.
- This was introduced in commit 119a6fc & 7d786b3 because of a potential
issue reported in #1573.
- The ACMP tree structure is initialized when the operator is
initialized.
- During transaction execution the ACMP tree structure is only 'read'
while traversing the tree (in acmp_process_quick) so this is safe for
use in a multi-threaded environment.
- Replaced pthread_mutex_t in modsecurity::operators::Pm with std::mutex
- Replaced pthread's thread usage in reading_logs_via_rule_message
example with std::thread.
- Simplified and modernized C++ code.
- Removed unnecessary includes of pthread.h
- Replaced WITHOUT_XXX build options with WITH_XXX to make it easier to
understand and configure.
- Updated GitHub workflow to align with these changes and include a
build 'with lmdb' (again, analogous to non-Windows configurations)
- As reported in #3054, the resolve methods in InMemoryPerProcess are
not acquiring a lock/mutex to prevent concurrent access to the data
structures that may be modified at the same time from other threads,
and thus triggering undefined behaviour.
- Replace inheritance of std::unordered_multimap data structure with
data member to prevent potential clients to use it without acquiring
the mutex to protect concurrent access.
- Replace pthreads lock with std C++11 std::shared_mutex
- Provides exclusive/shared lock access so that multiple readers can
access the data at the same time, but only one writer. this is used
to favor query performance by allowing more concurrent access to the
data until an update needs to be performed.
- Simplifies acquisition and disposal of lock/mutex with
std::lock_guard, which has RAII semantics.
- NOTE: Because std::shared_mutex is not recursive, calls to another
function that tries to acquire the lock will fail. Introduced
__store & __updateFirst helper methods to workaround this.
- Updates to InMemoryPerProcess::resolveFirst
- Updated the code to store the expired var in 'expiredVars' to delete
them after iterating over the range (and releasing the read lock, as
'delIfExpired' needs to acquire it for exclusive access), as the
current call to 'delIfExpired' would invalidate the range triggering
undefined behaviour on the following iteration.
- Noticed that in commit 118e1b3 the call to 'delIfExpired' in this
function is done using 'it->second.getValue()'' instead of
'it->first', which seems incorrect (based on similar code in other
resolveXXX functions).
- Updated InMemoryPerProcess::delIfExpired to use 'std::find_if' (with a
lambda that matches both the key and the 'isExpired' condition)
because the data structure is a multimap. The version introduced in
commit 118e1b3 could find an entry (not necessarily the first, because
the map is unordered) where 'isExpired' is 'false' and exit, while
another entry could be expired.
- cppreference mentions this about the constructor that receives a
const char *:
- Constructs the string with the contents initialized with a copy of
the null-terminated character string pointed to by s. The length of
the string is determined by the first null character. The behavior
is undefined if [s, s + Traits::length(s)) is not a valid range
(for example, if s is a null pointer).
- C++23 introduces a deleted constructor to prevent this in static
scenarios, which is how this issue was detected.
- This configuration flag was introduced in commit d47185d in the
context of PR #3207.
- Moved to the configure step's 'run' command in order to be shared
across configurations.
- For the sake of reference, matrix.platform.configure should be used
for configuration flags that are needed for a specific
platform/architecture (which was the reason it was introduced in
commit d9255d8, PR #3144).
C++17 introduced a construct to create and initialize a variable within the
condition of if and switch statements, and C++20 added this construct to
range-based for loops. Using this new feature simplifies common code patterns
and helps in giving variables the right scope.
Reference: https://sonarcloud.io/project/issues?open=AZDCieK2zGtqRpL2rnl-&id=owasp-modsecurity_ModSecurity
- After the GitHub macOS runner images were upgraded to macOS 14.6
(Sonoma), the test 'Include remote rules - failed download (Abort)'
started failing because the error message reported by curl/OS is no
longer 'HTTP response code said error'.
- Initialize `log` temporary value on construction instead of doing
default initialization and then calling `append`.
- Leverage `std::string_view` to replace `const std::string&` parameters
in `utils::string::replaceAll` to avoid creating a `std::string`
object (and associated allocation and copy) for the string literal`%d`
- Keep m_it->disruptive value and use it as return value to guarantee
that the value is correct.
- If m_it->disruptive is false and the 'it' argument has not been
initialized/cleaned, the function may incorrectly return a non-zero
value.
- When a disruptive intervention is being reported by the function,
defensively initialize log & url to NULL if there's no such data to
provide to the caller.
- If the caller has not initialized/cleaned those fields in the 'it'
argument, after returning from transaction::intervention, the user
can safely read the log & url fields and in all scenarios they'll
have valid values.
