From bb07de9ad7e574edc24496c184730ca15fffe94e Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 28 Apr 2024 21:45:34 -0300 Subject: [PATCH 1/6] toupper/tolower is already receiving a copy, so it doesn't need to create a new one to transform it - Make functions inline to improve performance - Introduced helper method toCaseHelper to remove code duplication --- src/utils/string.cc | 29 ----------------------------- src/utils/string.h | 27 ++++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/utils/string.cc b/src/utils/string.cc index 276fd31f53..195d5a1b87 100644 --- a/src/utils/string.cc +++ b/src/utils/string.cc @@ -20,12 +20,9 @@ #include #include -#include #include #include -#include #include -#include #include #include @@ -148,32 +145,6 @@ std::string toHexIfNeeded(const std::string &str, bool escape_spec) { } -std::string tolower(std::string str) { - std::string value; - value.resize(str.length()); - - std::transform(str.begin(), - str.end(), - value.begin(), - ::tolower); - - return value; -} - - -std::string toupper(std::string str) { - std::string value; - value.resize(str.length()); - - std::transform(str.begin(), - str.end(), - value.begin(), - ::toupper); - - return value; -} - - std::vector ssplit(std::string str, char delimiter) { std::vector internal; std::stringstream ss(str); // Turn the string into a stream. diff --git a/src/utils/string.h b/src/utils/string.h index 1cd63296bf..54136e4924 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -14,9 +14,10 @@ */ #include -#include #include #include +#include +#include #ifndef SRC_UTILS_STRING_H_ #define SRC_UTILS_STRING_H_ @@ -62,8 +63,6 @@ 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 ssplit(std::string str, char delimiter); std::pair ssplit_pair(const std::string& str, char delimiter); std::vector split(std::string str, char delimiter); @@ -77,6 +76,28 @@ unsigned char x2c(const unsigned char *what); unsigned char xsingle2c(const unsigned char *what); unsigned char *c2x(unsigned what, unsigned char *where); + +template +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 } // namespace modsecurity From f8dd09f7c9b740dadb6e10468263dab9162e8974 Mon Sep 17 00:00:00 2001 From: eduar-hte <130087371+eduar-hte@users.noreply.github.com> Date: Sat, 18 May 2024 16:55:41 +0000 Subject: [PATCH 2/6] Avoid creating a new std::string on the heap to create VariableValue - Introduced helper method addVariableOrigin to reduce code duplication. --- src/variables/rule.h | 65 +++++++++++++++------------------------- src/variables/variable.h | 5 ++-- src/variables/xml.cc | 5 ++-- 3 files changed, 28 insertions(+), 47 deletions(-) diff --git a/src/variables/rule.h b/src/variables/rule.h index 3d3cbcc05c..eed98c8ae9 100644 --- a/src/variables/rule.h +++ b/src/variables/rule.h @@ -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 *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 *l) { + auto var = new VariableValue(&m_rule, &key, + &value + ); + var->addOrigin(); + l->push_back(var); + } }; diff --git a/src/variables/variable.h b/src/variables/variable.h index e854aca713..fc4671bbbe 100644 --- a/src/variables/variable.h +++ b/src/variables/variable.h @@ -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; diff --git a/src/variables/xml.cc b/src/variables/xml.cc index 9b3d8ff96e..03dbc96732 100644 --- a/src/variables/xml.cc +++ b/src/variables/xml.cc @@ -124,13 +124,12 @@ void XML::evaluate(Transaction *t, content = reinterpret_cast( 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); } } From 1534ee24488b00decc7611f38fbbc95f7b9634ab Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Thu, 30 May 2024 16:32:16 +0000 Subject: [PATCH 3/6] Removed unnecessary copies --- src/request_body_processor/multipart.cc | 31 +++++++------------------ 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index d9ed6c6d15..93df5e7fa9 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -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; @@ -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; } From 8b17f3691fc618614daa8ff043f39d2ed3bed17c Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Fri, 24 May 2024 21:11:21 -0300 Subject: [PATCH 4/6] Inline string functions --- src/Makefile.am | 1 - src/utils/string.cc | 239 -------------------------------------------- src/utils/string.h | 213 +++++++++++++++++++++++++++++++++++---- 3 files changed, 194 insertions(+), 259 deletions(-) delete mode 100644 src/utils/string.cc diff --git a/src/Makefile.am b/src/Makefile.am index cf5549e75d..ef9357aac7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -253,7 +253,6 @@ UTILS = \ utils/random.cc \ utils/regex.cc \ utils/sha1.cc \ - utils/string.cc \ utils/system.cc \ utils/shared_files.cc diff --git a/src/utils/string.cc b/src/utils/string.cc deleted file mode 100644 index 195d5a1b87..0000000000 --- a/src/utils/string.cc +++ /dev/null @@ -1,239 +0,0 @@ -/* - * ModSecurity, http://www.modsecurity.org/ - * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/) - * - * You may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * If any of the files related to licensing are missing or if you have any - * other questions related to licensing please contact Trustwave Holdings, Inc. - * directly using the email address security@modsecurity.org. - * - */ - -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include - -#if defined _MSC_VER -#include -#elif defined __GNUC__ -#include -#include -#endif - -#include "modsecurity/modsecurity.h" - -#include "src/utils/string.h" - - -namespace modsecurity { -namespace utils { -namespace string { - - -std::string parserSanitizer(std::string a) { - a = removeWhiteSpacesIfNeeded(a); - a = removeBracketsIfNeeded(a); - return a; -} - - -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; -} - - -std::string ascTime(time_t *t) { - std::string ts = std::ctime(t); - ts.pop_back(); - return ts; -} - - -std::string dash_if_empty(const std::string *str) { - if (str == NULL || str->empty()) { - return "-"; - } - - return *str; -} - - -std::string dash_if_empty(const char *str) { - if (str == NULL || strlen(str) == 0) { - return "-"; - } - - return std::string(str); -} - - -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; -} - - -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; -} - - -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; -} - -std::string toHexIfNeeded(const std::string &str, bool escape_spec) { - // 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(); -} - - -std::vector ssplit(std::string str, char delimiter) { - std::vector 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; -} - - -std::pair 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); -} - - -std::vector split(std::string str, char delimiter) { - std::vector internal = ssplit(str, delimiter); - - if (internal.size() == 0) { - internal.push_back(str); - } - - return internal; -} - - -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); - } -} - - -unsigned char x2c(const unsigned char *what) { - unsigned char digit; - - digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0')); - digit *= 16; - digit += (what[1] >= 'A' ? ((what[1] & 0xdf) - 'A') + 10 : (what[1] - '0')); - - return digit; -} - - -/** - * Converts a single hexadecimal digit into a decimal value. - */ -unsigned char xsingle2c(const unsigned char *what) { - unsigned char digit; - - digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0')); - - return digit; -} - - -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; -} - - -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(); - } -} - -} // namespace string -} // namespace utils -} // namespace modsecurity diff --git a/src/utils/string.h b/src/utils/string.h index 54136e4924..d6495012f9 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -15,9 +15,12 @@ #include #include +#include #include #include #include +#include +#include #ifndef SRC_UTILS_STRING_H_ #define SRC_UTILS_STRING_H_ @@ -56,25 +59,197 @@ 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::vector ssplit(std::string str, char delimiter); -std::pair ssplit_pair(const std::string& str, char delimiter); -std::vector 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(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 == NULL || str->empty()) { + return "-"; + } + + return *str; +} + + +inline std::string dash_if_empty(const char *str) { + if (str == NULL || std::strlen(str) == 0) { + return "-"; + } + + return std::string(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 ssplit(std::string str, char delimiter) { + std::vector 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 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 split(std::string str, char delimiter) { + std::vector internal = ssplit(str, delimiter); + + if (internal.size() == 0) { + 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; +} + + +inline unsigned char x2c(const unsigned char *what) { + unsigned char digit; + + digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0')); + digit *= 16; + digit += (what[1] >= 'A' ? ((what[1] & 0xdf) - 'A') + 10 : (what[1] - '0')); + + return digit; +} + + +/** + * 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 *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 From cc0f893854545404b7b7172dcacfe26aab13e38c Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Fri, 9 Aug 2024 14:02:40 -0700 Subject: [PATCH 5/6] Removed unused overload of dash_if_empty that sonarcloud flags as potential buffer overflow --- src/transaction.cc | 4 ++-- src/utils/string.h | 9 --------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/transaction.cc b/src/transaction.cc index 7dc8ba2411..bf24e2ad28 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -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 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( diff --git a/src/utils/string.h b/src/utils/string.h index d6495012f9..eaacffe5c8 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -75,15 +75,6 @@ inline std::string dash_if_empty(const std::string *str) { } -inline std::string dash_if_empty(const char *str) { - if (str == NULL || std::strlen(str) == 0) { - return "-"; - } - - return std::string(str); -} - - inline std::string limitTo(int amount, const std::string &str) { std::string ret; From 77adb57524ccf803396d9d5cb3f3c17383cc0ccb Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Mon, 12 Aug 2024 06:18:20 -0700 Subject: [PATCH 6/6] Avoid std::string copy in ssplit argument - Other minor changes reported by sonarcloud --- src/request_body_processor/multipart.cc | 4 ++-- src/utils/string.h | 32 ++++++++++++------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index 93df5e7fa9..c578638bc7 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -855,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( @@ -924,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) { diff --git a/src/utils/string.h b/src/utils/string.h index eaacffe5c8..700f33b9dd 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -59,7 +59,7 @@ const char HEX2DEC[256] = { }; -inline std::string ascTime(time_t *t) { +inline std::string ascTime(const time_t *t) { std::string ts = std::ctime(t); ts.pop_back(); return ts; @@ -67,7 +67,7 @@ inline std::string ascTime(time_t *t) { inline std::string dash_if_empty(const std::string *str) { - if (str == NULL || str->empty()) { + if (str == nullptr || str->empty()) { return "-"; } @@ -107,7 +107,7 @@ inline std::string toHexIfNeeded(const std::string &str, bool escape_spec = fals } -inline std::vector ssplit(std::string str, char delimiter) { +inline std::vector ssplit(const std::string &str, char delimiter) { std::vector internal; std::stringstream ss(str); // Turn the string into a stream. std::string tok; @@ -134,10 +134,10 @@ inline std::pair ssplit_pair(const std::string& str, c } -inline std::vector split(std::string str, char delimiter) { +inline std::vector split(const std::string &str, char delimiter) { std::vector internal = ssplit(str, delimiter); - if (internal.size() == 0) { + if (internal.empty()) { internal.push_back(str); } @@ -145,10 +145,10 @@ inline std::vector split(std::string str, char delimiter) { } -inline void chomp(std::string *str) { - std::string::size_type pos = str->find_last_not_of("\n\r"); +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); + str.erase(pos+1, str.length()-pos-1); } } @@ -194,24 +194,24 @@ inline std::string parserSanitizer(std::string a) { } -inline unsigned char x2c(const unsigned char *what) { +/** + * 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')); - digit *= 16; - digit += (what[1] >= 'A' ? ((what[1] & 0xdf) - 'A') + 10 : (what[1] - '0')); return digit; } -/** - * Converts a single hexadecimal digit into a decimal value. - */ -inline unsigned char xsingle2c(const unsigned char *what) { +inline unsigned char x2c(const unsigned char *what) { unsigned char digit; - digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0')); + digit = xsingle2c(what); + digit *= 16; + digit += xsingle2c(what+1); return digit; }