Skip to content

Commit

Permalink
Minor codebase improvements suggested by Sonarcloud
Browse files Browse the repository at this point in the history
- 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.
  - Change this raw for-loop to a range for-loop or an "std::for_each".
  • Loading branch information
eduar-hte committed Sep 10, 2024
1 parent 147cee9 commit f9575ef
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 32 deletions.
4 changes: 2 additions & 2 deletions src/modsecurity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,13 @@ void ModSecurity::serverLog(void *data, const RuleMessage &rm) {

if (m_logProperties & TextLogProperty) {
auto d = rm.log();
const void *a = static_cast<const void *>(d.c_str());
auto a = static_cast<const void *>(d.c_str());
m_logCb(data, a);
return;
}

if (m_logProperties & RuleMessageLogProperty) {
const void *a = static_cast<const void *>(&rm);
auto a = static_cast<const void *>(&rm);
m_logCb(data, a);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,7 @@ std::string Transaction::toOldAuditLogFormat(int parts,
}
if (parts & audit_log::AuditLog::HAuditLogPart) {
audit_log << "--" << trailer << "-" << "H--" << std::endl;
for (auto a : m_rulesMessages) {
for (const auto &a : m_rulesMessages) {
audit_log << a.log(0, m_httpCodeReturned) << std::endl;
}
audit_log << std::endl;
Expand Down
7 changes: 2 additions & 5 deletions test/common/custom_debug_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@

namespace modsecurity_test
{

CustomDebugLog::~CustomDebugLog() {}

void CustomDebugLog::write(int level, const std::string &message)
{
m_log << "[" << level << "] " << message << std::endl;
Expand All @@ -39,14 +36,14 @@ namespace modsecurity_test
m_log << msgf << std::endl;
}

bool const CustomDebugLog::contains(const std::string &pattern) const
bool CustomDebugLog::contains(const std::string &pattern) const
{
modsecurity::Utils::Regex re(pattern);
std::string s = m_log.str();
return modsecurity::Utils::regex_search(s, re);
}

std::string const CustomDebugLog::log_messages() const
std::string CustomDebugLog::log_messages() const
{
return m_log.str();
}
Expand Down
5 changes: 2 additions & 3 deletions test/common/custom_debug_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ namespace modsecurity_test {
class CustomDebugLog : public modsecurity::debug_log::DebugLog {
public:
CustomDebugLog *new_instance();
~CustomDebugLog();

void write(int level, const std::string& message) override;
void write(int level, const std::string &id,
const std::string &uri, const std::string &msg) override;
bool const contains(const std::string& pattern) const;
std::string const log_messages() const;
bool contains(const std::string& pattern) const;
std::string log_messages() const;
std::string error_log_messages();
int getDebugLogLevel() override;

Expand Down
4 changes: 2 additions & 2 deletions test/common/modsecurity_test_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace modsecurity_test
private:
static void logCb(void *data, const void *msgv)
{
const char *msg = reinterpret_cast<const char *>(msgv);
std::stringstream *ss = (std::stringstream *)data;
auto msg = reinterpret_cast<const char *>(msgv);
auto ss = reinterpret_cast<std::stringstream *>(data);
*ss << msg << std::endl;
}
};
Expand Down
35 changes: 16 additions & 19 deletions test/regression/regression.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,17 @@ void actions(ModSecurityTestResults<RegressionTest> *r,
{
r->status = it.status;
}
if (it.url != NULL)
if (it.url != nullptr)
{
r->location.append(it.url);
free(it.url);
it.url = NULL;
it.url = nullptr;
}
if (it.log != NULL)
if (it.log != nullptr)
{
*serverLog << it.log;
free(it.log);
it.log = NULL;
it.log = nullptr;
}
}
}
Expand Down Expand Up @@ -329,10 +329,10 @@ void perform_unit_test(ModSecurityTest<RegressionTest> *test,

actions(&r, &modsec_transaction, &context.m_server_log);

for (const auto &headers : t->request_headers)
for (const auto &[name, value] : t->request_headers)
{
modsec_transaction.addRequestHeader(headers.first.c_str(),
headers.second.c_str());
modsec_transaction.addRequestHeader(name.c_str(),
value.c_str());
}

modsec_transaction.processRequestHeaders();
Expand All @@ -344,10 +344,10 @@ void perform_unit_test(ModSecurityTest<RegressionTest> *test,
modsec_transaction.processRequestBody();
actions(&r, &modsec_transaction, &context.m_server_log);

for (const auto &headers : t->response_headers)
for (const auto &[name, value] : t->response_headers)
{
modsec_transaction.addResponseHeader(headers.first.c_str(),
headers.second.c_str());
modsec_transaction.addResponseHeader(name.c_str(),
value.c_str());
}

modsec_transaction.processResponseHeaders(r.status,
Expand All @@ -362,7 +362,7 @@ void perform_unit_test(ModSecurityTest<RegressionTest> *test,

modsec_transaction.processLogging();

const auto *d = reinterpret_cast<CustomDebugLog *>(context.m_modsec_rules.m_debugLog);
const auto *d = static_cast<CustomDebugLog *>(context.m_modsec_rules.m_debugLog);

if (!d->contains(t->debug_log))
{
Expand Down Expand Up @@ -530,9 +530,9 @@ int main(int argc, char **argv)
int counter = 0;

std::list<std::string> keyList;
for (const auto &a : test)
for (const auto &[name, tests] : test)
{
keyList.push_back(a.first);
keyList.push_back(name);
}
keyList.sort();

Expand Down Expand Up @@ -611,13 +611,10 @@ int main(int argc, char **argv)
std::cout << "disabled test(s)." << RESET << std::endl;
}

for (auto a : test)
for (auto &[name, vec] : test)
{
std::vector<RegressionTest *> *vec = a.second;
for (int i = 0; i < vec->size(); i++)
{
delete vec->at(i);
}
for (auto x : *vec)
delete x;
delete vec;
}

Expand Down

0 comments on commit f9575ef

Please sign in to comment.