- Marked the conversion operator in `Pcre2MatchContextPtr` as `explicit`
to improve type safety and prevent unintended implicit conversions.
- Ensured consistent use of `nullptr` instead of `NULL` for better readability and modern C++ compliance.
These changes enhance code clarity, maintainability, and adherence to modern C++ best practices.
Introduced PCRE2_CFLAGS, PCRE2_LDADD, and PCRE2_LDFLAGS in all relevant Makefile.am files to align with the existing PCRE_* variable usage. This change addresses potential issues with linking and configuration for builds on macOS GitHub runners.
These modifications aim to resolve the build failure observed exclusively in the macOS environment while maintaining compatibility across other platforms. Testing will confirm if this adjustment corrects the issue.
Updated the build system and related source files to use libpcre2 as the
default regex library instead of the deprecated libpcre. This change
ensures future compatibility and aligns with the library's maintenance status.
To build with the old libpcre, the `--with-pcre` configuration parameter
can be specified.
- Added a cppcheck-suppress directive to handle a false positive detected
by cppcheck 2.16.
- This change addresses an issue caused by the recent Homebrew update
on macOS CI runner, which upgraded cppcheck from version 2.15 to 2.16.
- This is correct because base class is initialized before members are
initialized.
- Removes cppcheck suppression by addressing reported issue.
- Leverage C++11's 'default member initializer' to initialize m_provider
& m_demandsPassword and address Sonarcloud issue.
- The following two warnings were generated after introducing the change
to instantiate the DigestImpl template with the address of mbedtls_md5
or mbedtls_sha1:
- warning: src/utils/sha1.h,62,error,danglingTemporaryLifetime,Using
pointer that is a temporary.
- warning: src/utils/sha1.h,60,style,constVariablePointer,Variable
'ret' can be declared as pointer to const
- See https://github.com/owasp-modsecurity/ModSecurity/pull/3231#issuecomment-2312511500
- src/modsecurity.cc
- Replace the redundant type with "auto".
- src/transaction.cc
- Avoid this unnecessary copy by using a "const" reference.
- test/common/custom_debug_log.cc
- Use "=default" instead of the default implementation of this special
member functions.
- Removed the unnecessary destructor override instead.
- Annotate this function with "override" or "final".
- Removed the unnecessary destructor override instead.
- Remove this "const" qualifier from the return type in all
declarations.
- test/common/modsecurity_test_context.h
- Replace the redundant type with "auto".
- test/regression/regression.cc
- Use the "nullptr" literal.
- Replace this declaration by a structured binding declaration.
- Replace "reinterpret_cast" with a safer operation.
- Avoids copying std::shared_ptr when lifetime of the RuleMessage
is controlled by the caller.
- The RuleMessage instance is created in RuleWithActions::evaluate and
then used to call the overloaded version of this method that is
specialized by subclasses.
- Once the call to the overloaded method returns, the std::shared_ptr
is destroyed as it's not stored by any of the callers, so it can
be replaced with a stack variable and avoid paying the cost of
copying the std::shared_ptr (and its control block that is
guaranteed to be thread-safe and thus is not a straightforward
pointer copy)
- Introduced RuleMessage::reset because this is required by
RuleWithActions::performLogging when it's not the 'last log', the rule
has multimatch and it's to be logged.
- The current version is creating allocating another instance of
RuleMessage on the heap to copy the Rule & Transaction related state
while all the other members in the RuleMessage are set to their
default values.
- The new version leverages the existent, unused and incomplete
function 'clean' (renamed as 'reset') to do this on the current
instance.
- Notice that the current code preserves the value of m_saveMessage,
so 'reset' provides an argument for the caller to control whether
this member should be reinitialized.
- 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.
