Skip to content

Commit

Permalink
Leverage std::string_view in Variable::stringMatchResolveMulti & Vari…
Browse files Browse the repository at this point in the history 
…able::stringMatchResolve

- Avoids copying std::string to compare colllection name (col)
  • Loading branch information
@eduar-hte
eduar-hte committed Oct 22, 2024
1 parent 09df813 commit c441f7d
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/engine/lua.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ int Lua::getvar(lua_State *L) {
z = const_cast<void *>(lua_topointer(L, -1));
t = reinterpret_cast<Transaction *>(z);

std::string var = variables::Variable::stringMatchResolve(t, varname);
auto var = variables::Variable::stringMatchResolve(t, varname);
applyTransformations(L, t, 2, var);

if (var.size() == 0) {
Expand Down
10 changes: 5 additions & 5 deletions src/utils/regex.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class Pcre2MatchContextPtr {
};
#endif

namespace modsecurity {
namespace Utils {
namespace modsecurity::Utils {


// Helper function to tell us if the current config indicates CRLF is a valid newline sequence
bool crlfIsNewline() {
Expand Down Expand Up @@ -87,7 +87,7 @@ bool crlfIsNewline() {
return crlf_is_newline;
}

Regex::Regex(const std::string& pattern_, bool ignoreCase)
Regex::Regex(std::string_view pattern_, bool ignoreCase)
: pattern(pattern_.empty() ? ".*" : pattern_) {
#if WITH_PCRE2
PCRE2_SPTR pcre2_pattern = reinterpret_cast<PCRE2_SPTR>(pattern.c_str());
Expand Down Expand Up @@ -427,5 +427,5 @@ RegexResult Regex::to_regex_result(int pcre_exec_result) const {
}
}

} // namespace Utils
} // namespace modsecurity

} // namespace modsecurity::Utils
2 changes: 1 addition & 1 deletion src/utils/regex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ struct SMatchCapture {

class Regex {
public:
explicit Regex(const std::string& pattern_, bool ignoreCase = false);
explicit Regex(std::string_view pattern_, bool ignoreCase = false);
~Regex();

// m_pc and m_pce can't be easily copied
Expand Down
53 changes: 26 additions & 27 deletions src/variables/variable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,8 @@ class n : public Variable { \
};


namespace modsecurity {
namespace modsecurity::variables {

class Transaction;
namespace variables {

class KeyExclusion {
public:
Expand All @@ -118,7 +116,7 @@ class KeyExclusionRegex : public KeyExclusion {
public:
explicit KeyExclusionRegex(const Utils::Regex &re)
: m_re(re.pattern, true) { }
explicit KeyExclusionRegex(const std::string &re)
explicit KeyExclusionRegex(std::string_view re)
: m_re(re, true) { }

~KeyExclusionRegex() override { }
Expand All @@ -133,8 +131,8 @@ class KeyExclusionRegex : public KeyExclusion {

class KeyExclusionString : public KeyExclusion {
public:
explicit KeyExclusionString(const std::string &a)
: m_key(utils::string::toupper(a)) { }
explicit KeyExclusionString(std::string_view a)
: m_key(utils::string::toupper(std::string(a))) { }

~KeyExclusionString() override { }

Expand Down Expand Up @@ -169,7 +167,7 @@ class KeyExclusions : public std::deque<std::unique_ptr<KeyExclusion>> {
class VariableMonkeyResolution {
public:
VariableMonkeyResolution () { }
static inline bool comp(const std::string &a, const std::string &b) {
static inline bool comp(std::string_view a, std::string_view b) {
return a.size() == b.size()
&& std::equal(a.begin(), a.end(), b.begin(),
[](char aa, char bb) {
Expand All @@ -178,24 +176,25 @@ class VariableMonkeyResolution {
}

static void stringMatchResolveMulti(Transaction *t,
const std::string &variable,
std::string_view variable,
std::vector<const VariableValue *> &l) {
size_t collection_delimiter_offset = variable.find(".");
auto collection_delimiter_offset = variable.find(".");
if (collection_delimiter_offset == std::string::npos) {
collection_delimiter_offset = variable.find(":");
}
std::string col; // collection name excluding individual variable specification
std::string_view col; // collection name excluding individual variable specification
// NOTE: cannot use std::string_view here because resolve
// needs to receive a std::string to call find
std::string var; // variable within the collection
if (collection_delimiter_offset == std::string::npos) {
col = variable;
} else {
col = std::string(variable, 0, collection_delimiter_offset);
var = std::string(variable, collection_delimiter_offset + 1,
variable.length() - (collection_delimiter_offset + 1));
}
col = variable.substr(0, collection_delimiter_offset);
var = variable.substr(collection_delimiter_offset + 1);
}

// First check if the request is for a collection of type AnchoredSetVariable
AnchoredSetVariable* anchoredSetVariable = NULL;
AnchoredSetVariable* anchoredSetVariable = nullptr;
if (comp(col, "ARGS")) {
anchoredSetVariable = &t->m_variableArgs;
} else if (comp(col, "ARGS_GET")) {
Expand Down Expand Up @@ -237,7 +236,7 @@ class VariableMonkeyResolution {
} else if (comp(col, "FILES_TMPNAMES")) {
anchoredSetVariable = &t->m_variableFilesTmpNames;
}
if (anchoredSetVariable != NULL) {
if (anchoredSetVariable != nullptr) {
if (collection_delimiter_offset == std::string::npos) {
anchoredSetVariable->resolve(l);
} else {
Expand All @@ -247,15 +246,15 @@ class VariableMonkeyResolution {
}

// Next check for collection of type AnchoredSetVariableTranslationProxy
AnchoredSetVariableTranslationProxy* anchoredSetVariableTranslationProxy = NULL;
AnchoredSetVariableTranslationProxy* anchoredSetVariableTranslationProxy = nullptr;
if (comp(col, "ARGS_NAMES")) {
anchoredSetVariableTranslationProxy = &t->m_variableArgsNames;
} else if (comp(col, "ARGS_GET_NAMES")) {
anchoredSetVariableTranslationProxy = &t->m_variableArgsGetNames;
} else if (comp(col, "ARGS_POST_NAMES")) {
anchoredSetVariableTranslationProxy = &t->m_variableArgsPostNames;
}
if (anchoredSetVariableTranslationProxy != NULL) {
if (anchoredSetVariableTranslationProxy != nullptr) {
if (collection_delimiter_offset == std::string::npos) {
anchoredSetVariableTranslationProxy->resolve(l);
} else {
Expand Down Expand Up @@ -379,9 +378,9 @@ class VariableMonkeyResolution {
}

static std::string stringMatchResolve(Transaction *t,
const std::string &variable) {
std::string_view variable) {
std::unique_ptr<std::string> vv;
size_t collection = variable.find(".");
auto collection = variable.find(".");
if (collection == std::string::npos) {
collection = variable.find(":");
}
Expand Down Expand Up @@ -509,9 +508,10 @@ class VariableMonkeyResolution {
throw std::invalid_argument("Variable not found.");
}
} else {
std::string col = std::string(variable, 0, collection);
std::string var = std::string(variable, collection + 1,
variable.length() - (collection + 1));
const auto col = variable.substr(0, collection);
// NOTE: cannot use std::string_view here because resolveFirst
// needs to receive a std::string to call find
const std::string var(variable.substr(collection + 1));
if (comp(col, "ARGS")) {
vv = t->m_variableArgs.resolveFirst(var);
} else if (comp(variable, "ARGS_NAMES")) {
Expand Down Expand Up @@ -580,9 +580,9 @@ class VariableMonkeyResolution {
}
}
if (vv == nullptr) {
return std::string("");
return {};
}
return std::string(*vv.get());
return *vv.get();
}
};

Expand Down Expand Up @@ -731,7 +731,6 @@ class VariableModificatorCount : public Variable {
};


} // namespace variables
} // namespace modsecurity
} // namespace modsecurity::variables

#endif // SRC_VARIABLES_VARIABLE_H_

0 comments on commit c441f7d

Please sign in to comment.