- Leverage delegating constructor to avoid code duplication between the
two available Transaction constructors.
- The constructor without 'id' argument delegates to the one that
receives it by providing `nullptr` as a value, which is used to
flag that an id needs to be generated.
- Simplified constructor by removing member initialization where the
default constructor will be invoked.
- Because the lifetime of the RuleMessage instances do not extend beyond
the lifetime of the enclosing RuleWithActions & Transaction,
RuleMessage can just reference it and simplify its definition.
- Additionally, make the references const to show that it doesn't modify it.
- Replace RuleMessage copy constructor with default implementations.
- Removed unused RuleMessage assignment operator (which cannot be implemented
now that it has reference members).
- Removed constructor from RuleMessage pointer.
- Addressed Sonarcloud suggestions: Do not use the constructor's
initializer list for data member "xxx". Use the in-class initializer
instead.
- The previous version of this function was doing three strdup copies
to parse the pm content. The updated version only copies the value
once (in order not to modify the Operator's m_param member variable),
and then performs the updates inline.
- Binary parsing was broken because digits were not compared as
characters.
- Fail parsing when an invalid hex character is found.
- Error message in parse_pm_content would reference freed memory if
accessed by caller. Removed anyway because it was unused.
- Some of the Transformation classes would initialize their Action's
action_kind using the default (using Transformation constructor
without an action_kind parameter).
- Others, however, would use that constructor and initialize action_kind
manually in their constructor, but setting the default value
(RunTimeBeforeMatchAttemptKind = 1), which was redundant.
- Removed unused Transformation constructor to specify action_kind.
- Converted Action::Kind into an 'enum class' to require using the enum
constants (instead of integer values, which are difficult to track in
the codebase and change)
- This function already expects these arguments not to be null pointers,
doesn't validate them and just dereference them.
- In order to make this explicit and enforced by the compiler, they're
now passed as references.
- utils::urldecode_nonstrict_inplace decodes inplace so key & value,
which are values returned by utils::string::ssplit_pair can be
just be modified and do not need to be copied.
- Updated signature of utils::urldecode_nonstrict_inplace, as its
two callers already have std::string values.
- Use std::string in UrlEncode transformation, instead of manually
memory management. This avoids an additional copy after completing
encoding by just swapping the encoded value and the input.
- Removed inplace helper function from the class, as it's only
referenced by the implementation.
- Validate buffer size before accessing data. The previous
implementation would only check that there was a character available
in the buffer but could continue processing/reading characters from
an hex representation without checking bounds.
- Removed inplace & mytolower helper functions from the class, as
they're only referenced by the implementation.
- Removed duplicate VALID_HEX & ISODIGIT macros, already in
src/utils/string.h.
- Renamed Transformation::evaluate to Transformation::transform to avoid
confusion with Action's overload methods.
- Updated Transformation::transform signature to receive the value by
reference and perform the transformation inline, if possible.
- Some transformations still need to use a temporary std::string to
perform their work, and then copy the result back.
- Made Transformation::transform methods const and updated Transaction
parameter to be const.
- Transaction parameter could not be removed because it's used by just
a single transformation, UrlDecodeUni.
- Removed std::string Action::evaluate(const std::string &exp,
Transaction *transaction); which was only implemented by
Transformation but was not used from the base class, but only after
downcasting to Transformation, so it can just be declared there (and
not pollute other actions with a default member implementation -that
does nothing- which is never called).
- 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
- 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.
