- removed compile_rules method. it is now possible to add rules at any

  time
- fixed a concurrency issue with RE2 where it has locked during specific
  regex patterns
- fixed a seg fault bug with libgit2 where access to specific field
  inside the git_commit object.

Author: Gal Ben David

README.md

@@ -80,8 +80,6 @@ grs.add_rule(
     match_whitelist_patterns=[],
     match_blacklist_patterns=[],
 )
-# Compiles the rules. Should be called only once after all the rules were added
-grs.compile_rules()
 
 # Add file extensions to ignore during the search
 grs.add_ignored_file_extension(

setup.py

@@ -5,7 +5,7 @@
 
 setuptools.setup(
     name='PyRepScan',
-    version='0.4.0',
+    version='0.4.1',
     author='Gal Ben David',
     author_email='[email protected]',
     url='https://github.com/intsights/PyRepScan',

src/git_repository_scanner.cpp

@@ -33,10 +33,6 @@ class GitRepositoryScanner {
         this->rules_manager.add_rule(name, match_pattern, match_whitelist_patterns, match_blacklist_patterns);
     }
 
-    void compile_rules() {
-        this->rules_manager.compile_rules();
-    }
-
     void add_ignored_file_extension(
         std::string file_extension
     ) {
@@ -89,23 +85,13 @@ class GitRepositoryScanner {
             return;
         }
 
-        const git_oid * current_commit_id = git_commit_id(current_commit);
-        char current_commit_id_string[41] = {0};
-        git_oid_fmt(current_commit_id_string, current_commit_id);
-
-        const git_signature * current_commit_author = git_commit_author(current_commit);
-        std::string current_commit_message = git_commit_message(current_commit);
-
-        git_time_t commit_time = git_commit_time(current_commit);
-
         if (current_commit_parent_count == 1) {
             git_commit_parent(&parent_commit, current_commit, 0);
             git_commit_tree(&current_git_tree, current_commit);
             git_commit_tree(&parent_git_tree, parent_commit);
 
             git_diff_tree_to_tree(&diff, git_repo, parent_git_tree, current_git_tree, NULL);
 
-            git_commit_free(current_commit);
             git_commit_free(parent_commit);
             git_tree_free(current_git_tree);
             git_tree_free(parent_git_tree);
@@ -114,7 +100,6 @@ class GitRepositoryScanner {
 
             git_diff_tree_to_tree(&diff, git_repo, NULL, current_git_tree, NULL);
 
-            git_commit_free(current_commit);
             git_tree_free(current_git_tree);
         }
 
@@ -136,25 +121,33 @@ class GitRepositoryScanner {
                 std::string content((const char *)git_blob_rawcontent(blob));
                 auto matches = this->rules_manager.scan_content(content);
                 if (matches.has_value()) {
-                    for (auto & match : matches.value()) {
-                        char new_file_oid[41] = {0};
-                        git_oid_fmt(new_file_oid, &delta->new_file.id);
+                    this->results_mutex.lock();
+
+                    const git_oid * current_commit_id = git_commit_id(current_commit);
+                    char current_commit_id_string[41] = {0};
+                    git_oid_fmt(current_commit_id_string, current_commit_id);
 
-                        std::string current_commit_author_name = "";
-                        if (nullptr != current_commit_author->name) {
-                            current_commit_author_name = current_commit_author->name;
-                        }
-                        std::string current_commit_author_email = "";
-                        if (nullptr != current_commit_author->email) {
-                            current_commit_author_email = current_commit_author->email;
-                        }
+                    const git_signature * current_commit_author = git_commit_author(current_commit);
+                    std::string current_commit_message = git_commit_message(current_commit);
 
-                        this->results_mutex.lock();
+                    char new_file_oid[41] = {0};
+                    git_oid_fmt(new_file_oid, &delta->new_file.id);
+
+                    std::string current_commit_author_name = "";
+                    if (nullptr != current_commit_author->name) {
+                        current_commit_author_name = current_commit_author->name;
+                    }
+                    std::string current_commit_author_email = "";
+                    if (nullptr != current_commit_author->email) {
+                        current_commit_author_email = current_commit_author->email;
+                    }
 
-                        std::tm * commit_time_tm = std::gmtime(&commit_time);
-                        std::ostringstream commit_time_ss;
-                        commit_time_ss << std::put_time(commit_time_tm, "%FT%T");
+                    git_time_t commit_time = git_commit_time(current_commit);
+                    std::tm * commit_time_tm = std::gmtime(&commit_time);
+                    std::ostringstream commit_time_ss;
+                    commit_time_ss << std::put_time(commit_time_tm, "%FT%T");
 
+                    for (auto & match : matches.value()) {
                         results.push_back(
                             {
                                 {"commit_id", std::string(current_commit_id_string)},
@@ -168,14 +161,15 @@ class GitRepositoryScanner {
                                 {"match", match["match"]},
                             }
                         );
-
-                        this->results_mutex.unlock();
                     }
+
+                    this->results_mutex.unlock();
                 }
                 git_blob_free(blob);
             }
         }
 
+        git_commit_free(current_commit);
         git_diff_free(diff);
     }
 
@@ -210,6 +204,8 @@ class GitRepositoryScanner {
         tf::Executor executor;
         executor.run(taskflow).wait();
 
+        git_repository_free(git_repo);
+
         return results;
     }
 
@@ -264,11 +260,6 @@ PYBIND11_MODULE(pyrepscan, m) {
             pybind11::arg("match_whitelist_patterns"),
             pybind11::arg("match_blacklist_patterns")
         )
-        .def(
-            "compile_rules",
-            &RulesManager::compile_rules,
-            ""
-        )
         .def(
             "add_ignored_file_extension",
             &RulesManager::add_ignored_file_extension,
@@ -313,15 +304,10 @@ PYBIND11_MODULE(pyrepscan, m) {
             pybind11::arg("repository_path"),
             pybind11::arg("branch_glob_pattern")
         )
-        .def(
-            "compile_rules",
-            &GitRepositoryScanner::compile_rules,
-            "Compile all the added rules to make them available for a scan.\nCall this function after you finished adding all the rules"
-        )
         .def(
             "add_rule",
             &GitRepositoryScanner::add_rule,
-            "Adding a rule to the list of rules.\nAfter a compile_rules call, no more rules can be added.",
+            "Adding a rule to the list of rules.",
             pybind11::arg("name"),
             pybind11::arg("match_pattern"),
             pybind11::arg("match_whitelist_patterns"),

src/rules_manager.hpp

@@ -71,15 +71,10 @@ class RulesManager {
             this->match_whitelist_pattern_set.push_back(match_whitelists_pattern_set);
         }
 
-        this->match_pattern_set.Add(match_pattern, NULL);
         this->match_patterns.push_back(std::make_shared<re2::RE2>(match_pattern));
         this->rule_names.push_back(name);
     }
 
-    void compile_rules() {
-        this->match_pattern_set.Compile();
-    }
-
     void add_ignored_file_extension(
         const std::string &file_extension
     ) {
@@ -114,39 +109,37 @@ class RulesManager {
     inline std::optional<std::vector<std::map<std::string, std::string>>> scan_content(
         const std::string &content
     ) {
-        std::vector<std::int32_t> matched_regexes;
+        std::vector<std::map<std::string, std::string>> matches;
+        std::string match;
 
-        if (this->match_pattern_set.Match(content, &matched_regexes)) {
-            std::vector<std::map<std::string, std::string>> matches;
-            std::string match;
+        for (std::uint32_t pattern_index = 0; pattern_index < this->match_patterns.size(); ++pattern_index) {
+            re2::StringPiece input(content);
+            while (re2::RE2::FindAndConsume(&input, *this->match_patterns[pattern_index], &match)) {
+                if (this->match_blacklist_pattern_set[pattern_index]->Match(match, NULL)) {
+                    continue;
+                }
 
-            for (const auto & match_index : matched_regexes) {
-                re2::StringPiece input(content);
-                while (re2::RE2::FindAndConsume(&input, *this->match_patterns[match_index], &match)) {
-                    if (this->match_blacklist_pattern_set[match_index]->Match(match, NULL)) {
-                        continue;
-                    }
+                if (
+                    this->match_whitelist_pattern_set[pattern_index] != nullptr &&
+                    !this->match_whitelist_pattern_set[pattern_index]->Match(match, NULL)
+                ) {
+                    continue;
+                }
 
-                    if (
-                        this->match_whitelist_pattern_set[match_index] != nullptr &&
-                        !this->match_whitelist_pattern_set[match_index]->Match(match, NULL)
-                    ) {
-                        continue;
+                matches.push_back(
+                    {
+                        {"rule_name", this->rule_names[pattern_index]},
+                        {"match", match},
                     }
-
-                    matches.push_back(
-                        {
-                            {"rule_name", this->rule_names[match_index]},
-                            {"match", match},
-                        }
-                    );
-                }
+                );
             }
+        }
 
+        if (matches.size() == 0) {
+            return std::nullopt;
+        } else {
             return matches;
         }
-
-        return std::nullopt;
     }
 
     inline std::vector<std::string> check_pattern(
@@ -178,8 +171,4 @@ class RulesManager {
     std::vector<std::shared_ptr<re2::RE2::Set>> match_whitelist_pattern_set;
     std::vector<std::shared_ptr<re2::RE2::Set>> match_blacklist_pattern_set;
     std::vector<std::shared_ptr<re2::RE2>> match_patterns;
-    re2::RE2::Set match_pattern_set = re2::RE2::Set(
-        re2::RE2::Quiet,
-        re2::RE2::UNANCHORED
-    );
 };

tests/test_pyrepscan.py

@@ -77,7 +77,6 @@ def test_add_rule_one(
             match_whitelist_patterns=[],
             match_blacklist_patterns=[],
         )
-        rules_manager.compile_rules()
 
         matches = rules_manager.scan_content(
             content='first line\nsecond line\nthird line',
@@ -125,7 +124,6 @@ def test_add_rule_two(
                 r'line',
             ],
         )
-        rules_manager.compile_rules()
 
         matches = rules_manager.scan_content(
             content='first line\nsecond line\nthird line',
@@ -164,7 +162,6 @@ def test_add_rule_three(
                 r'line',
             ],
         )
-        rules_manager.compile_rules()
 
         matches = rules_manager.scan_content(
             content='first line\nsecond line\nthird line',
@@ -359,7 +356,6 @@ def test_scan(
             match_whitelist_patterns=[],
             match_blacklist_patterns=[],
         )
-        grs.compile_rules()
 
         grs.add_ignored_file_extension('py')
         grs.add_ignored_file_path('test_')