Use std::string_view as key for collection::Collection

- Introduced transparent comparator & hash to be able to lookup for std::string_view in std::unordered_map where the key is a std::string, which avoids creating unnecessary std::string instances (with a heap-allocation and copy) - This is a C++20 feature, which introduced support for transparent comparisons in std::unordered_map. - Refactored code to remove duplication of MyEqual & MyHash - Replaced simple hash function with a variant of the Fowler-Noll-Vo hash function (FNV-1a)
eduar-hte · Oct 23, 2024 · f8cd6c5 · f8cd6c5
1 parent 1e2a8a7
commit f8cd6c5
Show file tree

Hide file tree

Showing 12 changed files with 229 additions and 219 deletions.
diff --git a/headers/modsecurity/anchored_set_variable.h b/headers/modsecurity/anchored_set_variable.h
@@ -30,6 +30,7 @@
 #endif
 
 #include "modsecurity/variable_value.h"
+#include "modsecurity/collection/util.h"
 
 #ifndef HEADERS_MODSECURITY_ANCHORED_SET_VARIABLE_H_
 #define HEADERS_MODSECURITY_ANCHORED_SET_VARIABLE_H_
@@ -46,49 +47,31 @@ class KeyExclusions;
 }
 
 
-struct MyEqual {
-    bool operator()(const std::string& Left, const std::string& Right) const {
-        return Left.size() == Right.size()
-             && std::equal(Left.begin(), Left.end(), Right.begin(),
-            [](char a, char b) {
-            return tolower(a) == tolower(b);
-        });
-    }
-};
-
-struct MyHash{
-    size_t operator()(const std::string& Keyval) const {
-        // You might need a better hash function than this
-        size_t h = 0;
-        std::for_each(Keyval.begin(), Keyval.end(), [&](char c) {
-            h += tolower(c);
-        });
-        return h;
-    }
-};
-
-
 class AnchoredSetVariable : public std::unordered_multimap<std::string,
 	VariableValue *, MyHash, MyEqual> {
  public:
     AnchoredSetVariable(Transaction *t, const std::string &name);
     ~AnchoredSetVariable();
 
+#if __cplusplus >= 202002L
+    using KeyType = std::string_view;
+#else
+    using KeyType = const std::string&;
+#endif
+
     void unset();
 
-    void set(const std::string &key, const std::string &value,
+    void set(KeyType key, std::string_view value,
         size_t offset);
 
-    void set(const std::string &key, const std::string &value,
+    void set(KeyType key, std::string_view value,
         size_t offset, size_t len);
 
-    void setCopy(std::string key, std::string value, size_t offset);
-
     void resolve(std::vector<const VariableValue *> &l);
     void resolve(std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke);
 
-    void resolve(const std::string &key,
+    void resolve(KeyType key,
         std::vector<const VariableValue *> &l);
 
     void resolveRegularExpression(Utils::Regex *r,
@@ -98,7 +81,7 @@ class AnchoredSetVariable : public std::unordered_multimap<std::string,
         std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke);
 
-    std::unique_ptr<std::string> resolveFirst(const std::string &key);
+    std::unique_ptr<std::string> resolveFirst(KeyType key);
 
     Transaction *m_transaction;
     std::string m_name;

diff --git a/headers/modsecurity/anchored_set_variable_translation_proxy.h b/headers/modsecurity/anchored_set_variable_translation_proxy.h
@@ -45,6 +45,12 @@ class AnchoredSetVariableTranslationProxy {
 
     virtual ~AnchoredSetVariableTranslationProxy() = default;
 
+#if __cplusplus >= 202002L
+    using KeyType = std::string_view;
+#else
+    using KeyType = const std::string&;
+#endif
+
     void resolve(std::vector<const VariableValue *> &l) {
         m_fount->resolve(l);
         translate(l);
@@ -56,7 +62,7 @@ class AnchoredSetVariableTranslationProxy {
         translate(l);
     }
 
-    void resolve(const std::string &key,
+    void resolve(KeyType key,
         std::vector<const VariableValue *> &l) {
         m_fount->resolve(key, l);
         translate(l);
@@ -75,7 +81,7 @@ class AnchoredSetVariableTranslationProxy {
         translate(l);
     };
 
-    std::unique_ptr<std::string> resolveFirst(const std::string &key) {
+    std::unique_ptr<std::string> resolveFirst(KeyType key) {
         std::vector<const VariableValue *> l;
         resolve(l);
 

diff --git a/headers/modsecurity/collection/collection.h b/headers/modsecurity/collection/collection.h
@@ -45,117 +45,123 @@ class Collection {
     explicit Collection(std::string_view a) : m_name(a) { }
     virtual ~Collection() { }
 
-    virtual bool storeOrUpdateFirst(const std::string& key,
+#if __cplusplus >= 202002L
+    using KeyType = std::string_view;
+#else
+    using KeyType = const std::string&;
+#endif
+
+    virtual bool storeOrUpdateFirst(KeyType key,
         std::string_view value) = 0;
 
-    virtual bool updateFirst(const std::string& key,
+    virtual bool updateFirst(KeyType key,
         std::string_view value) = 0;
 
-    virtual void del(const std::string& key) = 0;
+    virtual void del(KeyType key) = 0;
 
-    virtual void setExpiry(const std::string& key, int32_t expiry_seconds) = 0;
+    virtual void setExpiry(KeyType key, int32_t expiry_seconds) = 0;
 
     virtual std::unique_ptr<std::string> resolveFirst(
-        const std::string& var) = 0;
+        KeyType key) = 0;
 
-    virtual void resolveSingleMatch(const std::string& var,
+    virtual void resolveSingleMatch(KeyType key,
         std::vector<const VariableValue *> &l) = 0;
-    virtual void resolveMultiMatches(const std::string& var,
+    virtual void resolveMultiMatches(KeyType key,
         std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke) = 0;
-    virtual void resolveRegularExpression(const std::string& var,
+    virtual void resolveRegularExpression(KeyType key,
         std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke) = 0;
 
 
     /* storeOrUpdateFirst */
-    bool storeOrUpdateFirst(std::string_view key,
+    bool storeOrUpdateFirst(KeyType key,
         std::string_view compartment, std::string_view value) {
         return storeOrUpdateFirst(nkey(compartment, key), value);
     }
 
 
-    bool storeOrUpdateFirst(std::string_view key,
+    bool storeOrUpdateFirst(KeyType key,
         std::string_view compartment, std::string_view compartment2,
         std::string_view value) {
         return storeOrUpdateFirst(nkey(compartment, compartment2, key), value);
     }
 
 
     /* updateFirst */
-    bool updateFirst(std::string_view key, std::string_view compartment,
+    bool updateFirst(KeyType key, std::string_view compartment,
         std::string_view value) {
         return updateFirst(nkey(compartment, key), value);
     }
 
 
-    bool updateFirst(std::string_view key, std::string_view compartment,
+    bool updateFirst(KeyType key, std::string_view compartment,
         std::string_view compartment2, std::string_view value) {
         return updateFirst(nkey(compartment, compartment2, key), value);
     }
 
 
     /* del */
-    void del(std::string_view key, std::string_view compartment) {
+    void del(KeyType key, std::string_view compartment) {
         del(nkey(compartment, key));
     }
 
 
-    void del(std::string_view key, std::string_view compartment,
+    void del(KeyType key, std::string_view compartment,
         std::string_view compartment2) {
         del(nkey(compartment, compartment2, key));
     }
 
 
     /* setExpiry */
-    void setExpiry(std::string_view key, std::string_view compartment,
+    void setExpiry(KeyType key, std::string_view compartment,
         int32_t expiry_seconds) {
         setExpiry(nkey(compartment, key), expiry_seconds);
     }
 
 
-    void setExpiry(std::string_view key, std::string_view compartment,
+    void setExpiry(KeyType key, std::string_view compartment,
         std::string_view compartment2, int32_t expiry_seconds) {
         setExpiry(nkey(compartment, compartment2, key), expiry_seconds);
     }
 
 
     /* resolveFirst */
-    std::unique_ptr<std::string> resolveFirst(std::string_view var,
+    std::unique_ptr<std::string> resolveFirst(KeyType var,
         std::string_view compartment) {
         return resolveFirst(nkey(compartment, var));
     }
 
 
-    std::unique_ptr<std::string> resolveFirst(std::string_view var,
+    std::unique_ptr<std::string> resolveFirst(KeyType var,
         std::string_view compartment, std::string_view compartment2) {
         return resolveFirst(nkey(compartment, compartment2, var));
     }
 
 
     /* resolveSingleMatch */
-    void resolveSingleMatch(std::string_view var,
+    void resolveSingleMatch(KeyType var,
         std::string_view compartment, std::vector<const VariableValue *> &l) {
         resolveSingleMatch(nkey(compartment, var), l);
     }
 
 
-    void resolveSingleMatch(std::string_view var,
+    void resolveSingleMatch(KeyType var,
         std::string_view compartment, std::string_view compartment2,
         std::vector<const VariableValue *> &l) {
         resolveSingleMatch(nkey(compartment, compartment2, var), l);
     }
 
 
     /* resolveMultiMatches */
-    void resolveMultiMatches(std::string_view var,
+    void resolveMultiMatches(KeyType var,
         std::string_view compartment, std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke) {
         resolveMultiMatches(nkey(compartment, var), l, ke);
     }
 
 
-    void resolveMultiMatches(std::string_view var,
+    void resolveMultiMatches(KeyType var,
         std::string_view compartment, std::string_view compartment2,
         std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke) {
@@ -164,14 +170,14 @@ class Collection {
 
 
     /* resolveRegularExpression */
-    void resolveRegularExpression(std::string_view var,
+    void resolveRegularExpression(KeyType var,
         std::string_view compartment, std::vector<const VariableValue *> &l,
         variables::KeyExclusions &ke) {
         resolveRegularExpression(nkey(compartment, var), l, ke);
     }
 
 
-    void resolveRegularExpression(std::string_view var,
+    void resolveRegularExpression(KeyType var,
         std::string_view compartment, std::string_view compartment2,
         std::vector<const VariableValue *> &l, variables::KeyExclusions &ke) {
         resolveRegularExpression(nkey(compartment, compartment2, var), l, ke);

diff --git a/headers/modsecurity/collection/util.h b/headers/modsecurity/collection/util.h
@@ -0,0 +1,72 @@
+/*
+ * ModSecurity, http://www.modsecurity.org/
+ * Copyright (c) 2015 - 2021 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.
+ *
+ */
+
+#ifndef HEADERS_MODSECURITY_COLLECTION_UTIL_H_H
+#define HEADERS_MODSECURITY_COLLECTION_UTIL_H_H
+
+#ifdef __cplusplus
+#include <string>
+#include <algorithm>
+
+
+namespace modsecurity {
+
+
+struct MyEqual {
+#if __cplusplus >= 202002L
+    using is_transparent = void;
+
+    template<typename T, typename U>
+    bool operator()(const T& Left, const U& Right) const {
+#else
+    bool operator()(const std::string& Left, const std::string& Right) const {
+#endif
+        return Left.size() == Right.size()
+             && std::equal(Left.begin(), Left.end(), Right.begin(),
+            [](char a, char b) {
+            return tolower(a) == tolower(b);
+        });
+    }
+};
+
+
+struct MyHash{
+#if __cplusplus >= 202002L
+    using is_transparent = void;
+
+    template<typename T>
+    std::size_t operator()(const T& Keyval) const {
+#else
+    std::size_t operator()(const std::string& Keyval) const {
+#endif
+        // computes the hash using a variant of the
+        // Fowler-Noll-Vo hash function (FNV-1a)
+        constexpr std::uint64_t prime{0x01000193}; // FNV prime
+        std::size_t hash{0x811c9dc5}; // FNV offset basis
+        for (char c : Keyval) {
+            hash ^= tolower(c);
+            hash *= prime; 
+        }
+        return hash;
+    }
+};
+
+
+}  // namespace modsecurity
+
+
+#endif  // __cplusplus
+
+#endif  // HEADERS_MODSECURITY_COLLECTION_UTIL_H_H
diff --git a/src/anchored_set_variable.cc b/src/anchored_set_variable.cc
@@ -50,16 +50,16 @@ void AnchoredSetVariable::unset() {
 }
 
 
-void AnchoredSetVariable::set(const std::string &key,
-    const std::string &value, size_t offset, size_t len) {
+void AnchoredSetVariable::set(KeyType key,
+    std::string_view value, size_t offset, size_t len) {
     auto var = new VariableValue(m_name, key, value);
     var->addOrigin(len, offset);
     emplace(key, var);
 }
 
 
-void AnchoredSetVariable::set(const std::string &key,
-    const std::string &value, size_t offset) {
+void AnchoredSetVariable::set(KeyType key,
+    std::string_view value, size_t offset) {
     auto var = new VariableValue(m_name, key, value);
     var->addOrigin(value.size(), offset);
     emplace(key, var);
@@ -88,8 +88,9 @@ void AnchoredSetVariable::resolve(
 }
 
 
-void AnchoredSetVariable::resolve(const std::string &key,
+void AnchoredSetVariable::resolve(KeyType key,
     std::vector<const VariableValue *> &l) {
+
     auto range = this->equal_range(key);
     for (auto it = range.first; it != range.second; ++it) {
         l.push_back(new VariableValue(*it->second));
@@ -98,7 +99,7 @@ void AnchoredSetVariable::resolve(const std::string &key,
 
 
 std::unique_ptr<std::string> AnchoredSetVariable::resolveFirst(
-    const std::string &key) {
+    KeyType key) {
 
     if (auto search = this->find(key); search != this->end()) {
         return std::make_unique<std::string>(search->second->getValue());