Skip to content

Remove several string copies and unnecessary heap allocations #3222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 13, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
@@ -253,7 +253,6 @@ UTILS = \
utils/random.cc \
utils/regex.cc \
utils/sha1.cc \
utils/string.cc \
utils/system.cc \
utils/shared_files.cc

35 changes: 10 additions & 25 deletions src/request_body_processor/multipart.cc
Original file line number Diff line number Diff line change
@@ -70,22 +70,20 @@ void MultipartPartTmpFile::Open() {

localtime_r(&tt, &timeinfo);

char tstr[300] {};
strftime(tstr, 299, "/%Y%m%d-%H%M%S", &timeinfo);
char tstr[17];
strftime(tstr, std::size(tstr), "/%Y%m%d-%H%M%S", &timeinfo);

std::string path = m_transaction->m_rules->m_uploadDirectory.m_value;
path = path + tstr + "-" + *m_transaction->m_id.get();
path += "-file-XXXXXX";

char* tmp = strdup(path.c_str());
#ifndef WIN32
m_tmp_file_fd = mkstemp(tmp);
m_tmp_file_fd = mkstemp(path.data());
#else
_mktemp_s(tmp, path.length()+1);
m_tmp_file_fd = _open(tmp, _O_CREAT | _O_EXCL | _O_RDWR);
_mktemp_s(path.data(), path.length()+1);
m_tmp_file_fd = _open(path.c_str(), _O_CREAT | _O_EXCL | _O_RDWR);
#endif
m_tmp_file_name.assign(tmp);
free(tmp);
m_tmp_file_name = path;
ms_dbg_a(m_transaction, 4, "MultipartPartTmpFile: Create filename= " + m_tmp_file_name);

int mode = m_transaction->m_rules->m_uploadFileMode.m_value;
@@ -857,7 +855,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
}

new_value = std::string(data);
utils::string::chomp(&new_value);
utils::string::chomp(new_value);

/* update the header value in the table */
header_value = m_mpp->m_headers.at(
@@ -926,7 +924,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
i++;
}
header_value = std::string(data);
utils::string::chomp(&header_value);
utils::string::chomp(header_value);

/* error if the name already exists */
if (m_mpp->m_headers.count(header_name) > 0) {
@@ -1271,22 +1269,10 @@ int Multipart::multipart_complete(std::string *error) {


int Multipart::count_boundary_params(const std::string& str_header_value) {
std::string lower = utils::string::tolower(str_header_value);
const char *header_value = lower.c_str();
char *duplicate = NULL;
char *s = NULL;
int count = 0;

if (header_value == NULL) {
return -1;
}

duplicate = strdup(header_value);
if (duplicate == NULL) {
return -1;
}

s = duplicate;
const auto lower = utils::string::tolower(str_header_value);
const char *s = lower.c_str();
while ((s = strstr(s, "boundary")) != NULL) {
s += 8;

@@ -1295,7 +1281,6 @@ int Multipart::count_boundary_params(const std::string& str_header_value) {
}
}

free(duplicate);
return count;
}

4 changes: 2 additions & 2 deletions src/transaction.cc
Original file line number Diff line number Diff line change
@@ -1520,7 +1520,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
ss << utils::string::dash_if_empty(
m_variableRequestHeaders.resolveFirst("Host").get())
<< " ";
ss << utils::string::dash_if_empty(this->m_clientIpAddress->c_str()) << " ";
ss << utils::string::dash_if_empty(this->m_clientIpAddress.get()) << " ";
/** TODO: Check variable */
variables::RemoteUser *r = new variables::RemoteUser("REMOTE_USER");
std::vector<const VariableValue *> l;
@@ -1531,7 +1531,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
delete r;

ss << utils::string::dash_if_empty(
m_variableRemoteUser.c_str());
&m_variableRemoteUser);
ss << " ";
/** TODO: Check variable */
//ss << utils::string::dash_if_empty(
268 changes: 0 additions & 268 deletions src/utils/string.cc

This file was deleted.

231 changes: 209 additions & 22 deletions src/utils/string.h
Original file line number Diff line number Diff line change
@@ -14,9 +14,13 @@
*/

#include <ctime>
#include <iostream>
#include <string>
#include <cstring>
#include <vector>
#include <algorithm>
#include <utility>
#include <sstream>
#include <iomanip>

#ifndef SRC_UTILS_STRING_H_
#define SRC_UTILS_STRING_H_
@@ -55,27 +59,210 @@ const char HEX2DEC[256] = {
};


std::string ascTime(time_t *t);
std::string dash_if_empty(const char *str);
std::string dash_if_empty(const std::string *str);
std::string limitTo(int amount, const std::string &str);
std::string removeBracketsIfNeeded(std::string a);
std::string string_to_hex(const std::string& input);
std::string toHexIfNeeded(const std::string &str, bool escape_spec = false);
std::string tolower(std::string str);
std::string toupper(std::string str);
std::vector<std::string> ssplit(std::string str, char delimiter);
std::pair<std::string, std::string> ssplit_pair(const std::string& str, char delimiter);
std::vector<std::string> split(std::string str, char delimiter);
void chomp(std::string *str);
void replaceAll(std::string &str, std::string_view from,
std::string_view to);
std::string removeWhiteSpacesIfNeeded(std::string a);
std::string parserSanitizer(std::string a);

unsigned char x2c(const unsigned char *what);
unsigned char xsingle2c(const unsigned char *what);
unsigned char *c2x(unsigned what, unsigned char *where);
inline std::string ascTime(const time_t *t) {
std::string ts = std::ctime(t);
ts.pop_back();
return ts;
}


inline std::string dash_if_empty(const std::string *str) {
if (str == nullptr || str->empty()) {
return "-";
}

return *str;
}


inline std::string limitTo(int amount, const std::string &str) {
std::string ret;

if (str.length() > amount) {
ret.assign(str, 0, amount);
ret = ret + " (" + std::to_string(str.length() - amount) + " " \
"characters omitted)";
return ret;
}

return str;
}


inline std::string toHexIfNeeded(const std::string &str, bool escape_spec = false) {
// escape_spec: escape special chars or not
// spec chars: '"' (quotation mark, ascii 34), '\' (backslash, ascii 92)
std::stringstream res;

for (int i = 0; i < str.size(); i++) {
int c = (unsigned char)str.at(i);
if (c < 32 || c > 126 || (escape_spec == true && (c == 34 || c == 92))) {
res << "\\x" << std::setw(2) << std::setfill('0') << std::hex << c;
} else {
res << str.at(i);
}
}

return res.str();
}


inline std::vector<std::string> ssplit(const std::string &str, char delimiter) {
std::vector<std::string> internal;
std::stringstream ss(str); // Turn the string into a stream.
std::string tok;

while (getline(ss, tok, delimiter)) {
internal.push_back(tok);
}

return internal;
}


inline std::pair<std::string, std::string> ssplit_pair(const std::string& str, char delimiter) {
std::stringstream ss(str); // Turn the string into a stream.
std::string key;
std::string value;

getline(ss, key, delimiter);
if (key.length() < str.length()) {
value = str.substr(key.length()+1);
}

return std::make_pair(key, value);
}


inline std::vector<std::string> split(const std::string &str, char delimiter) {
std::vector<std::string> internal = ssplit(str, delimiter);

if (internal.empty()) {
internal.push_back(str);
}

return internal;
}


inline void chomp(std::string &str) {
std::string::size_type pos = str.find_last_not_of("\n\r");
if (pos != std::string::npos) {
str.erase(pos+1, str.length()-pos-1);
}
}


inline void replaceAll(std::string &str, std::string_view from,
std::string_view to) {
size_t start_pos = 0;
while ((start_pos = str.find(from, start_pos)) != std::string::npos) {
str.replace(start_pos, from.length(), to);
start_pos += to.length();
}
}


inline std::string removeWhiteSpacesIfNeeded(std::string a) {
while (a.size() > 1 && a.at(0) == ' ') {
a.erase(0, 1);
}
while (a.size() > 1 && a.at(a.length()-1) == ' ') {
a.pop_back();
}
return a;
}


inline std::string removeBracketsIfNeeded(std::string a) {
if (a.length() > 1 && a.at(0) == '"' && a.at(a.length()-1) == '"') {
a.pop_back();
a.erase(0, 1);
}
if (a.length() > 1 && a.at(0) == '\'' && a.at(a.length()-1) == '\'') {
a.pop_back();
a.erase(0, 1);
}
return a;
}


inline std::string parserSanitizer(std::string a) {
a = removeWhiteSpacesIfNeeded(a);
a = removeBracketsIfNeeded(a);
return a;
}


/**
* Converts a single hexadecimal digit into a decimal value.
*/
inline unsigned char xsingle2c(const unsigned char *what) {
unsigned char digit;

digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0'));

return digit;
}


inline unsigned char x2c(const unsigned char *what) {
unsigned char digit;

digit = xsingle2c(what);
digit *= 16;
digit += xsingle2c(what+1);

return digit;
}


inline unsigned char *c2x(unsigned what, unsigned char *where) {
static const char c2x_table[] = "0123456789abcdef";

what = what & 0xff;
*where++ = c2x_table[what >> 4];
*where++ = c2x_table[what & 0x0f];

return where;
}


inline std::string string_to_hex(const std::string& input) {
static const char* const lut = "0123456789ABCDEF";
size_t len = input.length();

std::string output;
output.reserve(2 * len);
for (size_t i = 0; i < len; ++i) {
const unsigned char c = input[i];
output.push_back(lut[c >> 4]);
output.push_back(lut[c & 15]);
}
return output;
}


template<typename Operation>
inline std::string toCaseHelper(std::string str, Operation op) {
std::transform(str.begin(),
str.end(),
str.begin(),
op);

return str;
}


inline std::string tolower(std::string str) { // cppcheck-suppress passedByValue ; copied value is used for in-place transformation
return toCaseHelper(str, ::tolower);
}


inline std::string toupper(std::string str) { // cppcheck-suppress passedByValue ; copied value is used for in-place transformation
return toCaseHelper(str, ::toupper);
}


} // namespace string
} // namespace utils
65 changes: 24 additions & 41 deletions src/variables/rule.h
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ namespace variables {
class Rule_DictElement : public VariableDictElement { \
public:
explicit Rule_DictElement(const std::string &dictElement)
: VariableDictElement(std::string("RULE"), dictElement) { }
: VariableDictElement(m_rule, dictElement) { }

static void id(Transaction *t,
RuleWithActions *rule,
@@ -49,13 +49,8 @@ class Rule_DictElement : public VariableDictElement { \
if (!r || r->m_ruleId == 0) {
return;
}
std::string *a = new std::string(std::to_string(r->m_ruleId));
VariableValue *var = new VariableValue(&m_rule, &m_rule_id,
a
);
delete a;
var->addOrigin();
l->push_back(var);

addVariableOrigin(m_rule_id, std::to_string(r->m_ruleId), l);
}


@@ -72,13 +67,7 @@ class Rule_DictElement : public VariableDictElement { \
return;
}

std::string *a = new std::string(r->m_rev);
VariableValue *var = new VariableValue(&m_rule, &m_rule_rev,
a
);
delete a;
var->addOrigin();
l->push_back(var);
addVariableOrigin(m_rule_rev, r->m_rev, l);
}


@@ -92,13 +81,7 @@ class Rule_DictElement : public VariableDictElement { \
}

if (r && r->hasSeverity()) {
std::string *a = new std::string(std::to_string(r->severity()));
VariableValue *var = new VariableValue(&m_rule, &m_rule_severity,
a
);
delete a;
var->addOrigin();
l->push_back(var);
addVariableOrigin(m_rule_severity, std::to_string(r->severity()), l);
}
}

@@ -113,13 +96,7 @@ class Rule_DictElement : public VariableDictElement { \
}

if (r && r->hasLogData()) {
std::string *a = new std::string(r->logData(t));
VariableValue *var = new VariableValue(&m_rule, &m_rule_logdata,
a
);
delete a;
var->addOrigin();
l->push_back(var);
addVariableOrigin(m_rule_logdata, r->logData(t), l);
}
}

@@ -133,36 +110,30 @@ class Rule_DictElement : public VariableDictElement { \
}

if (r && r->hasMsg()) {
std::string *a = new std::string(r->msg(t));
VariableValue *var = new VariableValue(&m_rule, &m_rule_msg,
a
);
delete a;
var->addOrigin();
l->push_back(var);
addVariableOrigin(m_rule_msg, r->msg(t), l);
}
}

void evaluate(Transaction *t,
RuleWithActions *rule,
std::vector<const VariableValue *> *l) override {
if (m_dictElement == "id") {
if (m_dictElement == m_rule_id) {
id(t, rule, l);
return;
}
if (rule && m_dictElement == "rev") {
if (rule && m_dictElement == m_rule_rev) {
rev(t, rule, l);
return;
}
if (rule && m_dictElement == "severity") {
if (rule && m_dictElement == m_rule_severity) {
severity(t, rule, l);
return;
}
if (m_dictElement == "logdata") {
if (m_dictElement == m_rule_logdata) {
logData(t, rule, l);
return;
}
if (m_dictElement == "msg") {
if (m_dictElement == m_rule_msg) {
msg(t, rule, l);
return;
}
@@ -174,6 +145,18 @@ class Rule_DictElement : public VariableDictElement { \
static const std::string m_rule_severity;
static const std::string m_rule_logdata;
static const std::string m_rule_msg;

private:

static inline void addVariableOrigin(const std::string &key,
const std::string &value,
std::vector<const VariableValue *> *l) {
auto var = new VariableValue(&m_rule, &key,
&value
);
var->addOrigin();
l->push_back(var);
}
};


5 changes: 2 additions & 3 deletions src/variables/variable.h
Original file line number Diff line number Diff line change
@@ -707,9 +707,8 @@ class VariableModificatorCount : public Variable {
}
reslIn.clear();

std::string *res = new std::string(std::to_string(count));
val = new VariableValue(m_fullName.get(), res);
delete res;
auto res = std::to_string(count);
val = new VariableValue(m_fullName.get(), &res);

l->push_back(val);
return;
5 changes: 2 additions & 3 deletions src/variables/xml.cc
Original file line number Diff line number Diff line change
@@ -124,13 +124,12 @@ void XML::evaluate(Transaction *t,
content = reinterpret_cast<char *>(
xmlNodeGetContent(nodes->nodeTab[i]));
if (content != NULL) {
std::string *a = new std::string(content);
auto a = std::string(content);
VariableValue *var = new VariableValue(m_fullName.get(),
a);
&a);
if (!m_keyExclusion.toOmit(*m_fullName)) {
l->push_back(var);
}
delete a;
xmlFree(content);
}
}