From cc8e0feb8e4f853a0e81bd73713d7f1ae7a37a0e Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 25 Jan 2024 15:59:23 -0800 Subject: [PATCH 01/12] Change the Regex interface to better support use cases in ATS --- include/tsutil/Regex.h | 24 +++++++++------------- src/proxy/http/remap/UrlRewrite.cc | 4 ++-- src/tsutil/Regex.cc | 33 ++++++++++++++---------------- 3 files changed, 27 insertions(+), 34 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index a1c51e3661e..a5ffbd95878 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -28,8 +28,6 @@ #include #include -#include "swoc/MemSpan.h" - /// Match flags for regular expression evaluation. enum REFlags { RE_CASE_INSENSITIVE = 0x0001, ///< Ignore case (default: case sensitive). @@ -61,28 +59,26 @@ class Regex */ bool compile(const char *pattern, unsigned flags = 0); - /** Execute the regular expression. + /** Compile the @a pattern into a regular expression. * - * @param str String to match against. - * @return @c true if the pattern matched, @a false if not. + * @param pattern Source pattern for regular expression (null terminated). + * @param flags Compilation flags. + * @param error Pointer to string to receive error message. + * @param erroffset Pointer to integer to receive error offset. + * @return @a true if compiled successfully, @a false otherwise. * - * It is safe to call this method concurrently on the same instance of @a this. + * @a flags should be the bitwise @c or of @c REFlags values. */ - bool exec(std::string_view const &str) const; + bool compile(const char *pattern, const char **error, int *erroffset, unsigned flags = 0); /** Execute the regular expression. * * @param str String to match against. - * @param ovector Capture results. - * @param ovecsize Number of elements in @a ovector. * @return @c true if the pattern matched, @a false if not. * * It is safe to call this method concurrently on the same instance of @a this. - * - * Each capture group takes 3 elements of @a ovector, therefore @a ovecsize must - * be a multiple of 3 and at least three times the number of desired capture groups. */ - bool exec(std::string_view const &str, int *ovector, int ovecsize) const; + bool exec(std::string_view const &str) const; /** Execute the regular expression. * @@ -96,7 +92,7 @@ class Regex * Each capture group takes 3 elements of @a ovector, therefore @a ovecsize must * be a multiple of 3 and at least three times the number of desired capture groups. */ - bool exec(std::string_view str, swoc::MemSpan groups) const; + int exec(std::string_view const &str, int *ovector, int ovecsize) const; /// @return The number of groups captured in the last call to @c exec. int get_capture_count(); diff --git a/src/proxy/http/remap/UrlRewrite.cc b/src/proxy/http/remap/UrlRewrite.cc index 1f8176e97ca..f7a6a007e60 100644 --- a/src/proxy/http/remap/UrlRewrite.cc +++ b/src/proxy/http/remap/UrlRewrite.cc @@ -960,10 +960,10 @@ UrlRewrite::_regexMappingLookup(RegexMappingList ®ex_mappings, URL *request_u } int matches_info[MAX_REGEX_SUBS * 3]; - bool match_result = + int match_result = list_iter->regular_expression.exec(std::string_view(request_host, request_host_len), matches_info, countof(matches_info)); - if (match_result == true) { + if (match_result > 0) { Debug("url_rewrite_regex", "Request URL host [%.*s] matched regex in mapping of rank %d " "with %d possible substitutions", diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 42d9d27c286..8723e3f58e9 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -92,8 +92,14 @@ Regex::compile(const char *pattern, const unsigned flags) { const char *error = nullptr; int erroffset = 0; - int options = 0; - int study_opts = 0; + return this->compile(pattern, &error, &erroffset, flags); +} + +bool +Regex::compile(const char *pattern, const char **error, int *erroffset, const unsigned flags) +{ + int options = 0; + int study_opts = 0; if (regex) { return false; @@ -107,7 +113,7 @@ Regex::compile(const char *pattern, const unsigned flags) options |= PCRE_ANCHORED; } - regex = pcre_compile(pattern, options, &error, &erroffset, nullptr); + regex = pcre_compile(pattern, options, error, erroffset, nullptr); if (error) { regex = nullptr; return false; @@ -117,7 +123,7 @@ Regex::compile(const char *pattern, const unsigned flags) study_opts |= PCRE_STUDY_JIT_COMPILE; #endif - regex_extra = pcre_study(as_pcre(regex), study_opts, &error); + regex_extra = pcre_study(as_pcre(regex), study_opts, error); #ifdef PCRE_CONFIG_JIT if (regex_extra) { @@ -142,24 +148,15 @@ Regex::get_capture_count() bool Regex::exec(std::string_view const &str) const { - std::array ovector = {{0}}; - return this->exec(str, ovector); + int ovector[DEFAULT_GROUP_COUNT * 3]; + int rval = this->exec(str, ovector, DEFAULT_GROUP_COUNT * 3); + return rval > 0; } -bool +int Regex::exec(std::string_view const &str, int *ovector, int ovecsize) const { - int rv; - - rv = pcre_exec(as_pcre(regex), as_extra(regex_extra), str.data(), static_cast(str.size()), 0, 0, ovector, ovecsize); - return rv > 0; -} - -bool -Regex::exec(std::string_view str, swoc::MemSpan groups) const -{ - return 0 < - pcre_exec(as_pcre(regex), as_extra(regex_extra), str.data(), int(str.size()), 0, 0, groups.data(), int(groups.count())); + return pcre_exec(as_pcre(regex), as_extra(regex_extra), str.data(), static_cast(str.size()), 0, 0, ovector, ovecsize); } Regex::~Regex() From ed9a5050c1f8974fa793418293ff3d1d743d98bf Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 26 Jan 2024 11:26:06 -0800 Subject: [PATCH 02/12] Fixed checking errors on compile --- src/tsutil/Regex.cc | 2 +- src/tsutil/unit_tests/test_Regex.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 8723e3f58e9..20c032cee8e 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -114,7 +114,7 @@ Regex::compile(const char *pattern, const char **error, int *erroffset, const un } regex = pcre_compile(pattern, options, error, erroffset, nullptr); - if (error) { + if (*error != nullptr) { regex = nullptr; return false; } diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index 16f327dff67..4f1a522c224 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -47,7 +47,7 @@ TEST_CASE("Regex", "[libts][Regex]") { for (auto &item : test_data) { Regex r; - r.compile(item.regex.data()); + REQUIRE(r.compile(item.regex.data()) == true); for (auto &test : item.tests) { REQUIRE(r.exec(test.subject.data()) == test.match); From 1c5dc56921f3fc9b2e07644f035d7f10e534fce0 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Wed, 7 Feb 2024 08:06:56 -0800 Subject: [PATCH 03/12] coverting to pcre - work in progress --- include/tsutil/Regex.h | 34 ++++- src/tsutil/CMakeLists.txt | 5 +- src/tsutil/Regex.cc | 259 ++++++++++++++++++++++++-------------- 3 files changed, 195 insertions(+), 103 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index a5ffbd95878..fb06f889b7b 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -28,6 +28,13 @@ #include #include +#define PCRE2_CODE_UNIT_WIDTH 8 +#if __has_include() +#include +#else +#include +#endif + /// Match flags for regular expression evaluation. enum REFlags { RE_CASE_INSENSITIVE = 0x0001, ///< Ignore case (default: case sensitive). @@ -35,6 +42,22 @@ enum REFlags { RE_ANCHORED = 0x0004, ///< Anchored (Regex defaults to unanchored). }; +//---------------------------------------------------------------------------- +class RegexMatches +{ +public: + RegexMatches(uint32_t size = 20); + ~RegexMatches(); + + pcre2_match_data *get_match_data(); + void set_subject(std::string_view subject); + std::string_view operator[](size_t index) const; + +private: + pcre2_match_data *_match_data = nullptr; + std::string_view _subject; +}; + /** Wrapper for PCRE evaluation. * */ @@ -57,7 +80,7 @@ class Regex * * @a flags should be the bitwise @c or of @c REFlags values. */ - bool compile(const char *pattern, unsigned flags = 0); + bool compile(std::string_view, uint32_t flags = 0); /** Compile the @a pattern into a regular expression. * @@ -69,7 +92,7 @@ class Regex * * @a flags should be the bitwise @c or of @c REFlags values. */ - bool compile(const char *pattern, const char **error, int *erroffset, unsigned flags = 0); + bool compile(std::string_view pattern, std::string &error, int &erroffset, unsigned flags = 0); /** Execute the regular expression. * @@ -78,7 +101,7 @@ class Regex * * It is safe to call this method concurrently on the same instance of @a this. */ - bool exec(std::string_view const &str) const; + bool exec(const std::string_view &subject) const; /** Execute the regular expression. * @@ -92,7 +115,7 @@ class Regex * Each capture group takes 3 elements of @a ovector, therefore @a ovecsize must * be a multiple of 3 and at least three times the number of desired capture groups. */ - int exec(std::string_view const &str, int *ovector, int ovecsize) const; + int exec(const std::string_view &subject, RegexMatches &matches) const; /// @return The number of groups captured in the last call to @c exec. int get_capture_count(); @@ -102,8 +125,7 @@ class Regex // enough to use as pointers. For some reason the header defines in name only a struct and // then aliases it to the standard name, rather than simply declare the latter in name only. // The goal is completely wrap PCRE and not include that header in client code. - void *regex = nullptr; ///< Compiled expression. - void *regex_extra = nullptr; ///< Extra information about the expression. + pcre2_code *_code = nullptr; }; /** Deterministic Finite state Automata container. diff --git a/src/tsutil/CMakeLists.txt b/src/tsutil/CMakeLists.txt index 44b83448fdb..c2538fabfb3 100644 --- a/src/tsutil/CMakeLists.txt +++ b/src/tsutil/CMakeLists.txt @@ -50,9 +50,12 @@ add_library( ts_unit_parser.cc Regex.cc ) + +pkg_check_modules(PCRE2 REQUIRED IMPORTED_TARGET libpcre2-8) + add_library(ts::tsutil ALIAS tsutil) set_target_properties(tsutil PROPERTIES POSITION_INDEPENDENT_CODE TRUE PUBLIC_HEADER "${TSUTIL_PUBLIC_HEADERS}") -target_link_libraries(tsutil PUBLIC libswoc::libswoc yaml-cpp::yaml-cpp PCRE::PCRE) +target_link_libraries(tsutil PUBLIC libswoc::libswoc yaml-cpp::yaml-cpp PkgConfig::PCRE2) install( TARGETS tsutil diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 20c032cee8e..e6fca52e74f 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -26,151 +26,218 @@ #include #include -#if __has_include() -#include -#else -#include -#endif - +//---------------------------------------------------------------------------- namespace { -inline pcre * -as_pcre(void *p) +void * +my_malloc(size_t size, void * /*caller*/) { - return static_cast(p); + void *ptr = malloc(size); + return ptr; } -inline pcre_extra * -as_extra(void *p) + +void +my_free(void *ptr, void * /*caller*/) { - return static_cast(p); + free(ptr); } } // namespace -#ifdef PCRE_CONFIG_JIT -/* -Using two thread locals avoids the deadlock because without the thread local object access, get_jit_stack doesn't call -the TLS init function which ends up calling __cxx_thread_atexit(which locks the dl_whatever mutex). Since the raw -pointer doesn't have a destructor to call, it doesn't need to call this. Interestingly, get_jit_stack was calling the -TLS init function to setup the destructor call at thread exit whether or not the class was declared in the function -body. -*/ -namespace +//---------------------------------------------------------------------------- +class RegexContext { -thread_local pcre_jit_stack *jit_stack; - -struct JitStackCleanup { - ~JitStackCleanup() +public: + RegexContext() { - if (jit_stack) { - pcre_jit_stack_free(jit_stack); + _general_context = pcre2_general_context_create(my_malloc, my_free, nullptr); + _compile_context = pcre2_compile_context_create(_general_context); + _match_context = pcre2_match_context_create(_general_context); + _jit_stack = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); // 1 page min and 1MB max + pcre2_jit_stack_assign(_match_context, nullptr, _jit_stack); + } + ~RegexContext() + { + if (_general_context) { + pcre2_general_context_free(_general_context); + } + if (_compile_context) { + pcre2_compile_context_free(_compile_context); + } + if (_match_context) { + pcre2_match_context_free(_match_context); + } + if (_jit_stack) { + pcre2_jit_stack_free(_jit_stack); } } + pcre2_general_context * + get_general_context() + { + return _general_context; + } + pcre2_compile_context * + get_compile_context() + { + return _compile_context; + } + pcre2_match_context * + get_match_context() + { + return _match_context; + } + +private: + pcre2_general_context *_general_context = nullptr; + pcre2_compile_context *_compile_context = nullptr; + pcre2_match_context *_match_context = nullptr; + pcre2_jit_stack *_jit_stack = nullptr; }; -thread_local JitStackCleanup jsc; +//---------------------------------------------------------------------------- +namespace +{ +thread_local RegexContext global_context; +// pcre2_match_data* cast_match_data(void *match_data) { +// return reinterpret_cast(match_data); +// } +// pcre2_code* cast_code(void *code) { +// return reinterpret_cast(code); +// } +}; // namespace + +//---------------------------------------------------------------------------- +RegexMatches::RegexMatches(uint32_t size) +{ + _match_data = pcre2_match_data_create(size, global_context.get_general_context()); +} -pcre_jit_stack * -get_jit_stack(void *) +//---------------------------------------------------------------------------- +RegexMatches::~RegexMatches() { - if (!jit_stack) { - jit_stack = pcre_jit_stack_alloc(4096, 1024 * 1024); // 1 page min and 1MB max + if (_match_data) { + pcre2_match_data_free(_match_data); } - return jit_stack; } -} // end anonymous namespace -#endif // def PCRE_CONFIG_JIT - -Regex::Regex(Regex &&that) noexcept : regex(that.regex), regex_extra(that.regex_extra) +//---------------------------------------------------------------------------- +pcre2_match_data * +RegexMatches::get_match_data() { - that.regex = nullptr; - that.regex_extra = nullptr; + return _match_data; } -bool -Regex::compile(const char *pattern, const unsigned flags) +//---------------------------------------------------------------------------- +void +RegexMatches::set_subject(std::string_view subject) { - const char *error = nullptr; - int erroffset = 0; - return this->compile(pattern, &error, &erroffset, flags); + _subject = subject; } -bool -Regex::compile(const char *pattern, const char **error, int *erroffset, const unsigned flags) +//---------------------------------------------------------------------------- +std::string_view +RegexMatches::operator[](size_t index) const { - int options = 0; - int study_opts = 0; - - if (regex) { - return false; + // check if the index is valid + if (index >= pcre2_get_ovector_count(_match_data)) { + return std::string_view(); } - if (flags & RE_CASE_INSENSITIVE) { - options |= PCRE_CASELESS; - } + PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(_match_data); + return std::string_view(_subject.data() + ovector[2 * index], ovector[2 * index + 1] - ovector[2 * index]); +} - if (flags & RE_ANCHORED) { - options |= PCRE_ANCHORED; - } +//---------------------------------------------------------------------------- +Regex::Regex(Regex &&that) noexcept +{ + _code = that._code; + that._code = nullptr; +} - regex = pcre_compile(pattern, options, error, erroffset, nullptr); - if (*error != nullptr) { - regex = nullptr; - return false; +//---------------------------------------------------------------------------- +Regex::~Regex() +{ + if (_code) { + pcre2_code_free(_code); } +} -#ifdef PCRE_CONFIG_JIT - study_opts |= PCRE_STUDY_JIT_COMPILE; -#endif - - regex_extra = pcre_study(as_pcre(regex), study_opts, error); - -#ifdef PCRE_CONFIG_JIT - if (regex_extra) { - pcre_assign_jit_stack(as_extra(regex_extra), &get_jit_stack, nullptr); - } -#endif +//---------------------------------------------------------------------------- +bool +Regex::compile(std::string_view pattern, uint32_t flags) +{ + std::string error; + int erroroffset; - return true; + return this->compile(pattern, error, erroroffset, flags); } -int -Regex::get_capture_count() +//---------------------------------------------------------------------------- +bool +Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, uint32_t flags) { - int captures = -1; - if (pcre_fullinfo(as_pcre(regex), as_extra(regex_extra), PCRE_INFO_CAPTURECOUNT, &captures) != 0) { - return -1; + if (_code) { + pcre2_code_free(_code); + } + PCRE2_SIZE error_offset; + int error_code; + _code = pcre2_compile(reinterpret_cast(pattern.data()), pattern.size(), flags, &error_code, &error_offset, + global_context.get_compile_context()); + if (!_code) { + erroroffset = error_offset; + + // get pcre2 error message + PCRE2_UCHAR buffer[256]; + pcre2_get_error_message(error_code, buffer, sizeof(buffer)); + error.assign((char *)buffer); + return false; } - return captures; + // support for JIT + pcre2_jit_compile(_code, PCRE2_JIT_COMPLETE); + + return true; } +//---------------------------------------------------------------------------- bool -Regex::exec(std::string_view const &str) const +Regex::exec(const std::string_view &subject) const { - int ovector[DEFAULT_GROUP_COUNT * 3]; - int rval = this->exec(str, ovector, DEFAULT_GROUP_COUNT * 3); - return rval > 0; + if (!_code) { + return false; + } + int rc = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, nullptr, nullptr); + return rc >= 0; } -int -Regex::exec(std::string_view const &str, int *ovector, int ovecsize) const +//---------------------------------------------------------------------------- +int32_t +Regex::exec(const std::string_view &subject, RegexMatches &matcher) const { - return pcre_exec(as_pcre(regex), as_extra(regex_extra), str.data(), static_cast(str.size()), 0, 0, ovector, ovecsize); + if (!_code) { + return 0; + } + int count = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, matcher.get_match_data(), + global_context.get_match_context()); + if (count < 0) { + return count; + } + + if (count > 0) { + matcher.set_subject(subject); + } + + return count; } -Regex::~Regex() +//---------------------------------------------------------------------------- +int +Regex::get_capture_count() { - if (regex_extra) { -#ifdef PCRE_CONFIG_JIT - pcre_free_study(as_extra(regex_extra)); -#else - pcre_free(regex_extra); -#endif - } - if (regex) { - pcre_free(regex); + int captures = -1; + if (pcre2_pattern_info(_code, PCRE2_INFO_CAPTURECOUNT, &captures) != 0) { + return -1; } + return captures; } DFA::~DFA() {} From d569d2533a201a9f7818c7b95e2f4efef085242b Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 8 Feb 2024 11:25:40 -0800 Subject: [PATCH 04/12] everything builds --- include/proxy/http/remap/UrlRewrite.h | 2 +- include/tsutil/Regex.h | 1 + src/proxy/http/remap/UrlRewrite.cc | 12 ++++++------ src/tsutil/Regex.cc | 19 ++++++++++++++----- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/include/proxy/http/remap/UrlRewrite.h b/include/proxy/http/remap/UrlRewrite.h index 86dcb50a07c..797ad94c472 100644 --- a/include/proxy/http/remap/UrlRewrite.h +++ b/include/proxy/http/remap/UrlRewrite.h @@ -232,7 +232,7 @@ class UrlRewrite : public RefCountObj int request_host_len); bool _regexMappingLookup(RegexMappingList ®ex_mappings, URL *request_url, int request_port, const char *request_host, int request_host_len, int rank_ceiling, UrlMappingContainer &mapping_container); - int _expandSubstitutions(int *matches_info, const RegexMapping *reg_map, const char *matched_string, char *dest_buf, + int _expandSubstitutions(size_t *matches_info, const RegexMapping *reg_map, const char *matched_string, char *dest_buf, int dest_buf_size); void _destroyTable(std::unique_ptr &h_table); void _destroyList(RegexMappingList ®exes); diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index fb06f889b7b..383e0a14ac3 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -52,6 +52,7 @@ class RegexMatches pcre2_match_data *get_match_data(); void set_subject(std::string_view subject); std::string_view operator[](size_t index) const; + size_t *get_ovector_pointer(); private: pcre2_match_data *_match_data = nullptr; diff --git a/src/proxy/http/remap/UrlRewrite.cc b/src/proxy/http/remap/UrlRewrite.cc index f7a6a007e60..448bf251d60 100644 --- a/src/proxy/http/remap/UrlRewrite.cc +++ b/src/proxy/http/remap/UrlRewrite.cc @@ -857,7 +857,7 @@ UrlRewrite::_mappingLookup(MappingsStore &mappings, URL *request_url, int reques // does not null terminate return string int -UrlRewrite::_expandSubstitutions(int *matches_info, const RegexMapping *reg_map, const char *matched_string, char *dest_buf, +UrlRewrite::_expandSubstitutions(size_t *matches_info, const RegexMapping *reg_map, const char *matched_string, char *dest_buf, int dest_buf_size) { int cur_buf_size = 0; @@ -959,9 +959,8 @@ UrlRewrite::_regexMappingLookup(RegexMappingList ®ex_mappings, URL *request_u continue; } - int matches_info[MAX_REGEX_SUBS * 3]; - int match_result = - list_iter->regular_expression.exec(std::string_view(request_host, request_host_len), matches_info, countof(matches_info)); + RegexMatches matches; + int match_result = list_iter->regular_expression.exec(std::string_view(request_host, request_host_len), matches); if (match_result > 0) { Debug("url_rewrite_regex", @@ -975,8 +974,9 @@ UrlRewrite::_regexMappingLookup(RegexMappingList ®ex_mappings, URL *request_u int buf_len; // Expand substitutions in the host field from the stored template - buf_len = _expandSubstitutions(matches_info, list_iter, request_host, buf, sizeof(buf)); - URL *expanded_url = mapping_container.createNewToURL(); + size_t *matches_info = matches.get_ovector_pointer(); + buf_len = _expandSubstitutions(matches_info, list_iter, request_host, buf, sizeof(buf)); + URL *expanded_url = mapping_container.createNewToURL(); expanded_url->copy(&((list_iter->url_map)->toURL)); expanded_url->host_set(buf, buf_len); diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index e6fca52e74f..baa4fce25e7 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -119,6 +119,13 @@ RegexMatches::~RegexMatches() } } +//---------------------------------------------------------------------------- +size_t * +RegexMatches::get_ovector_pointer() +{ + return pcre2_get_ovector_pointer(_match_data); +} + //---------------------------------------------------------------------------- pcre2_match_data * RegexMatches::get_match_data() @@ -205,25 +212,27 @@ Regex::exec(const std::string_view &subject) const if (!_code) { return false; } - int rc = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, nullptr, nullptr); - return rc >= 0; + RegexMatches matches; + + int count = this->exec(subject, matches); + return count > 0; } //---------------------------------------------------------------------------- int32_t -Regex::exec(const std::string_view &subject, RegexMatches &matcher) const +Regex::exec(const std::string_view &subject, RegexMatches &matches) const { if (!_code) { return 0; } - int count = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, matcher.get_match_data(), + int count = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, matches.get_match_data(), global_context.get_match_context()); if (count < 0) { return count; } if (count > 0) { - matcher.set_subject(subject); + matches.set_subject(subject); } return count; From fad492ece9c65a490d387d2eeff20f28e5343630 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 8 Feb 2024 14:11:08 -0800 Subject: [PATCH 05/12] Update the regex flags --- include/tsutil/Regex.h | 6 +++--- src/tsutil/Regex.cc | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 383e0a14ac3..2d224cf14c3 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -37,9 +37,9 @@ /// Match flags for regular expression evaluation. enum REFlags { - RE_CASE_INSENSITIVE = 0x0001, ///< Ignore case (default: case sensitive). - RE_UNANCHORED = 0x0002, ///< Unanchored (DFA defaults to anchored). - RE_ANCHORED = 0x0004, ///< Anchored (Regex defaults to unanchored). + RE_CASE_INSENSITIVE = PCRE2_CASELESS, ///< Ignore case (default: case sensitive). + RE_UNANCHORED = PCRE2_MULTILINE, ///< Unanchored (DFA defaults to anchored). + RE_ANCHORED = PCRE2_ANCHORED, ///< Anchored (Regex defaults to unanchored). }; //---------------------------------------------------------------------------- diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index baa4fce25e7..16ffcbac283 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -261,7 +261,7 @@ DFA::build(std::string_view const &pattern, unsigned flags) flags |= RE_ANCHORED; } - if (!rxp.compile(string.c_str(), flags)) { + if (!rxp.compile(pattern, flags)) { return false; } _patterns.emplace_back(std::move(rxp), std::move(string)); From faa8b863dd81a9e070abb8ed2dff694b4c749590 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 15 Feb 2024 12:31:47 -0800 Subject: [PATCH 06/12] Update to fix deadlock issues with dlopen and DbgCtl --- src/proxy/http/remap/UrlRewrite.cc | 2 +- src/tsutil/Regex.cc | 42 ++++++++++++++++++------------ 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/proxy/http/remap/UrlRewrite.cc b/src/proxy/http/remap/UrlRewrite.cc index 448bf251d60..e377451921d 100644 --- a/src/proxy/http/remap/UrlRewrite.cc +++ b/src/proxy/http/remap/UrlRewrite.cc @@ -908,6 +908,7 @@ UrlRewrite::_regexMappingLookup(RegexMappingList ®ex_mappings, URL *request_u int request_host_len, int rank_ceiling, UrlMappingContainer &mapping_container) { bool retval = false; + RegexMatches matches; if (rank_ceiling == -1) { // we will now look at all regex mappings rank_ceiling = INT_MAX; @@ -959,7 +960,6 @@ UrlRewrite::_regexMappingLookup(RegexMappingList ®ex_mappings, URL *request_u continue; } - RegexMatches matches; int match_result = list_iter->regular_expression.exec(std::string_view(request_host, request_host_len), matches); if (match_result > 0) { diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 16ffcbac283..41aa030f382 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -47,13 +47,13 @@ my_free(void *ptr, void * /*caller*/) class RegexContext { public: - RegexContext() + static RegexContext * + get_instance() { - _general_context = pcre2_general_context_create(my_malloc, my_free, nullptr); - _compile_context = pcre2_compile_context_create(_general_context); - _match_context = pcre2_match_context_create(_general_context); - _jit_stack = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); // 1 page min and 1MB max - pcre2_jit_stack_assign(_match_context, nullptr, _jit_stack); + if (!_regex_context) { + _regex_context = new RegexContext(); + } + return _regex_context; } ~RegexContext() { @@ -87,28 +87,36 @@ class RegexContext } private: + RegexContext() + { + _general_context = pcre2_general_context_create(my_malloc, my_free, nullptr); + _compile_context = pcre2_compile_context_create(_general_context); + _match_context = pcre2_match_context_create(_general_context); + _jit_stack = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); // 1 page min and 1MB max + pcre2_jit_stack_assign(_match_context, nullptr, _jit_stack); + } pcre2_general_context *_general_context = nullptr; pcre2_compile_context *_compile_context = nullptr; pcre2_match_context *_match_context = nullptr; pcre2_jit_stack *_jit_stack = nullptr; + thread_local static RegexContext *_regex_context; }; +thread_local RegexContext *RegexContext::_regex_context = nullptr; + //---------------------------------------------------------------------------- namespace { -thread_local RegexContext global_context; -// pcre2_match_data* cast_match_data(void *match_data) { -// return reinterpret_cast(match_data); -// } -// pcre2_code* cast_code(void *code) { -// return reinterpret_cast(code); -// } -}; // namespace +struct RegexContextCleanup { + ~RegexContextCleanup() { delete RegexContext::get_instance(); } +}; +thread_local RegexContextCleanup cleanup; +} // namespace //---------------------------------------------------------------------------- RegexMatches::RegexMatches(uint32_t size) { - _match_data = pcre2_match_data_create(size, global_context.get_general_context()); + _match_data = pcre2_match_data_create(size, RegexContext::get_instance()->get_general_context()); } //---------------------------------------------------------------------------- @@ -188,7 +196,7 @@ Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, u PCRE2_SIZE error_offset; int error_code; _code = pcre2_compile(reinterpret_cast(pattern.data()), pattern.size(), flags, &error_code, &error_offset, - global_context.get_compile_context()); + RegexContext::get_instance()->get_compile_context()); if (!_code) { erroroffset = error_offset; @@ -226,7 +234,7 @@ Regex::exec(const std::string_view &subject, RegexMatches &matches) const return 0; } int count = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, matches.get_match_data(), - global_context.get_match_context()); + RegexContext::get_instance()->get_match_context()); if (count < 0) { return count; } From 5b8e7efffc658cf0c19c0ee772095bd3bf454c3c Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 16 Feb 2024 14:09:32 -0800 Subject: [PATCH 07/12] cleaned up passing by value instead of passing by reference for string_view --- include/tsutil/Regex.h | 10 +++++----- src/tsutil/Regex.cc | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 2d224cf14c3..686047045ff 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -102,7 +102,7 @@ class Regex * * It is safe to call this method concurrently on the same instance of @a this. */ - bool exec(const std::string_view &subject) const; + bool exec(std::string_view subject) const; /** Execute the regular expression. * @@ -116,7 +116,7 @@ class Regex * Each capture group takes 3 elements of @a ovector, therefore @a ovecsize must * be a multiple of 3 and at least three times the number of desired capture groups. */ - int exec(const std::string_view &subject, RegexMatches &matches) const; + int exec(std::string_view subject, RegexMatches &matches) const; /// @return The number of groups captured in the last call to @c exec. int get_capture_count(); @@ -141,7 +141,7 @@ class DFA ~DFA(); /// @return The number of patterns successfully compiled. - int compile(std::string_view const &pattern, unsigned flags = 0); + int compile(std::string_view pattern, unsigned flags = 0); /// @return The number of patterns successfully compiled. int compile(std::string_view *patterns, int npatterns, unsigned flags = 0); /// @return The number of patterns successfully compiled. @@ -152,7 +152,7 @@ class DFA * @param str String to match. * @return Index of the matched pattern, -1 if no match. */ - int match(std::string_view const &str) const; + int match(std::string_view str) const; private: struct Pattern { @@ -167,7 +167,7 @@ class DFA * @param flags Regular expression compilation flags. * @return @c true if @a pattern was successfully compiled, @c false if not. */ - bool build(std::string_view const &pattern, unsigned flags = 0); + bool build(std::string_view pattern, unsigned flags = 0); std::vector _patterns; }; diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 41aa030f382..55e820a8ed4 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -215,7 +215,7 @@ Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, u //---------------------------------------------------------------------------- bool -Regex::exec(const std::string_view &subject) const +Regex::exec(std::string_view subject) const { if (!_code) { return false; @@ -228,7 +228,7 @@ Regex::exec(const std::string_view &subject) const //---------------------------------------------------------------------------- int32_t -Regex::exec(const std::string_view &subject, RegexMatches &matches) const +Regex::exec(std::string_view subject, RegexMatches &matches) const { if (!_code) { return 0; @@ -260,7 +260,7 @@ Regex::get_capture_count() DFA::~DFA() {} bool -DFA::build(std::string_view const &pattern, unsigned flags) +DFA::build(const std::string_view pattern, unsigned flags) { Regex rxp; std::string string{pattern}; @@ -277,7 +277,7 @@ DFA::build(std::string_view const &pattern, unsigned flags) } int -DFA::compile(std::string_view const &pattern, unsigned flags) +DFA::compile(std::string_view pattern, unsigned flags) { assert(_patterns.empty()); this->build(pattern, flags); @@ -305,7 +305,7 @@ DFA::compile(const char **patterns, int npatterns, unsigned flags) } int -DFA::match(std::string_view const &str) const +DFA::match(std::string_view str) const { for (auto spot = _patterns.begin(), limit = _patterns.end(); spot != limit; ++spot) { if (spot->_re.exec(str)) { From 8b5960aa503ac9b68fab895c1e74879381a0357b Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 16 Feb 2024 14:17:30 -0800 Subject: [PATCH 08/12] Updated comments in the code --- include/tsutil/Regex.h | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 686047045ff..6496beec55d 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -42,7 +42,7 @@ enum REFlags { RE_ANCHORED = PCRE2_ANCHORED, ///< Anchored (Regex defaults to unanchored). }; -//---------------------------------------------------------------------------- +/// @brief Wrapper for PCRE2 match data. class RegexMatches { public: @@ -59,9 +59,7 @@ class RegexMatches std::string_view _subject; }; -/** Wrapper for PCRE evaluation. - * - */ +/// @brief Wrapper for PCRE2 regular expression. class Regex { public: @@ -81,14 +79,14 @@ class Regex * * @a flags should be the bitwise @c or of @c REFlags values. */ - bool compile(std::string_view, uint32_t flags = 0); + bool compile(std::string_view pattern, uint32_t flags = 0); /** Compile the @a pattern into a regular expression. * * @param pattern Source pattern for regular expression (null terminated). - * @param flags Compilation flags. - * @param error Pointer to string to receive error message. + * @param error String to receive error message. * @param erroffset Pointer to integer to receive error offset. + * @param flags Compilation flags. * @return @a true if compiled successfully, @a false otherwise. * * @a flags should be the bitwise @c or of @c REFlags values. @@ -97,7 +95,7 @@ class Regex /** Execute the regular expression. * - * @param str String to match against. + * @param subject String to match against. * @return @c true if the pattern matched, @a false if not. * * It is safe to call this method concurrently on the same instance of @a this. @@ -106,10 +104,9 @@ class Regex /** Execute the regular expression. * - * @param str String to match against. - * @param ovector Capture results. - * @param ovecsize Number of elements in @a ovector. - * @return @c true if the pattern matched, @a false if not. + * @param subject String to match against. + * @param matches Place to store the capture groups. + * @return @c The number of capture groups. < 0 if an error occurred. 0 if the number of Matches is too small. * * It is safe to call this method concurrently on the same instance of @a this. * @@ -118,7 +115,7 @@ class Regex */ int exec(std::string_view subject, RegexMatches &matches) const; - /// @return The number of groups captured in the last call to @c exec. + /// @return The number of capture groups in the compiled pattern. int get_capture_count(); private: From b9a0f795a79211bfd0c06078bc7f883eef66396a Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 16 Feb 2024 14:35:00 -0800 Subject: [PATCH 09/12] More cleanup of the code and comments --- include/tsutil/Regex.h | 27 ++++++++++++++++++++------- src/tsutil/Regex.cc | 6 ++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 6496beec55d..fb641e7c48b 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -35,7 +35,7 @@ #include #endif -/// Match flags for regular expression evaluation. +/// @brief Match flags for regular expression evaluation. enum REFlags { RE_CASE_INSENSITIVE = PCRE2_CASELESS, ///< Ignore case (default: case sensitive). RE_UNANCHORED = PCRE2_MULTILINE, ///< Unanchored (DFA defaults to anchored). @@ -45,15 +45,31 @@ enum REFlags { /// @brief Wrapper for PCRE2 match data. class RegexMatches { + friend class Regex; + public: - RegexMatches(uint32_t size = 20); + /** Construct a new RegexMatches object. + * + * @param size The number of matches to allocate space for. + */ + RegexMatches(uint32_t size = 10); ~RegexMatches(); - pcre2_match_data *get_match_data(); - void set_subject(std::string_view subject); + /** Get the match at the given index. + * + * @return The match at the given index. + */ std::string_view operator[](size_t index) const; + /** Get the ovector pointer for the capture groups. Don't use this unless you know what you are doing. + * + * @return ovector pointer. + */ size_t *get_ovector_pointer(); +protected: + pcre2_match_data *get_match_data(); + void set_subject(std::string_view subject); + private: pcre2_match_data *_match_data = nullptr; std::string_view _subject; @@ -63,9 +79,6 @@ class RegexMatches class Regex { public: - /// Default number of capture groups. - static constexpr size_t DEFAULT_GROUP_COUNT = 10; - Regex() = default; Regex(Regex const &) = delete; // No copying. Regex(Regex &&that) noexcept; diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 55e820a8ed4..25f5924c23f 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -257,8 +257,10 @@ Regex::get_capture_count() return captures; } +//---------------------------------------------------------------------------- DFA::~DFA() {} +//---------------------------------------------------------------------------- bool DFA::build(const std::string_view pattern, unsigned flags) { @@ -276,6 +278,7 @@ DFA::build(const std::string_view pattern, unsigned flags) return true; } +//---------------------------------------------------------------------------- int DFA::compile(std::string_view pattern, unsigned flags) { @@ -284,6 +287,7 @@ DFA::compile(std::string_view pattern, unsigned flags) return _patterns.size(); } +//---------------------------------------------------------------------------- int DFA::compile(std::string_view *patterns, int npatterns, unsigned flags) { @@ -294,6 +298,7 @@ DFA::compile(std::string_view *patterns, int npatterns, unsigned flags) return _patterns.size(); } +//---------------------------------------------------------------------------- int DFA::compile(const char **patterns, int npatterns, unsigned flags) { @@ -304,6 +309,7 @@ DFA::compile(const char **patterns, int npatterns, unsigned flags) return _patterns.size(); } +//---------------------------------------------------------------------------- int DFA::match(std::string_view str) const { From 80bb4bedf739718e6261d72620ac2a2b99b0844d Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Wed, 21 Feb 2024 10:43:31 -0800 Subject: [PATCH 10/12] Added support for Matches to use the stack Added more unit testing --- include/tsutil/Regex.h | 17 ++-- src/tsutil/Regex.cc | 50 ++++++++++-- src/tsutil/unit_tests/test_Regex.cc | 119 ++++++++++++++++++++++++++-- 3 files changed, 170 insertions(+), 16 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index fb641e7c48b..75a27c142e7 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -52,7 +52,7 @@ class RegexMatches * * @param size The number of matches to allocate space for. */ - RegexMatches(uint32_t size = 10); + RegexMatches(uint32_t size = DEFAULT_MATCHES); ~RegexMatches(); /** Get the match at the given index. @@ -65,14 +65,21 @@ class RegexMatches * @return ovector pointer. */ size_t *get_ovector_pointer(); + int32_t size() const; protected: pcre2_match_data *get_match_data(); void set_subject(std::string_view subject); + void set_size(int32_t size); private: + constexpr static uint32_t DEFAULT_MATCHES = 10; + static void *malloc(size_t size, void *caller); pcre2_match_data *_match_data = nullptr; std::string_view _subject; + char _buffer[24 + 96 + 16 * DEFAULT_MATCHES]; // 24 bytes for the general context, 96 bytes overhead, 16 bytes per match. + size_t _buffer_bytes_used = 0; + int32_t _size = 0; }; /// @brief Wrapper for PCRE2 regular expression. @@ -151,18 +158,18 @@ class DFA ~DFA(); /// @return The number of patterns successfully compiled. - int compile(std::string_view pattern, unsigned flags = 0); + int32_t compile(std::string_view pattern, unsigned flags = 0); /// @return The number of patterns successfully compiled. - int compile(std::string_view *patterns, int npatterns, unsigned flags = 0); + int32_t compile(std::string_view *patterns, int npatterns, unsigned flags = 0); /// @return The number of patterns successfully compiled. - int compile(const char **patterns, int npatterns, unsigned flags = 0); + int32_t compile(const char **patterns, int npatterns, unsigned flags = 0); /** Match @a str against the internal patterns. * * @param str String to match. * @return Index of the matched pattern, -1 if no match. */ - int match(std::string_view str) const; + int32_t match(std::string_view str) const; private: struct Pattern { diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 25f5924c23f..e077e30886a 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -116,7 +116,28 @@ thread_local RegexContextCleanup cleanup; //---------------------------------------------------------------------------- RegexMatches::RegexMatches(uint32_t size) { - _match_data = pcre2_match_data_create(size, RegexContext::get_instance()->get_general_context()); + pcre2_general_context *ctx = pcre2_general_context_create( + &RegexMatches::malloc, [](void *, void *) -> void {}, static_cast(this)); + + _match_data = pcre2_match_data_create(size, ctx); +} + +//---------------------------------------------------------------------------- +void * +RegexMatches::malloc(size_t size, void *caller) +{ + auto *matches = static_cast(caller); + + // allocate from the buffer if possible + if (size <= sizeof(matches->_buffer) - matches->_buffer_bytes_used) { + void *ptr = matches->_buffer + matches->_buffer_bytes_used; + matches->_buffer_bytes_used += size; + return ptr; + } + + // otherwise use system malloc if the buffer is too small + void *ptr = ::malloc(size); + return ptr; } //---------------------------------------------------------------------------- @@ -134,6 +155,13 @@ RegexMatches::get_ovector_pointer() return pcre2_get_ovector_pointer(_match_data); } +//---------------------------------------------------------------------------- +int32_t +RegexMatches::size() const +{ + return _size; +} + //---------------------------------------------------------------------------- pcre2_match_data * RegexMatches::get_match_data() @@ -141,6 +169,13 @@ RegexMatches::get_match_data() return _match_data; } +//---------------------------------------------------------------------------- +void +RegexMatches::set_size(int32_t size) +{ + _size = size; +} + //---------------------------------------------------------------------------- void RegexMatches::set_subject(std::string_view subject) @@ -235,6 +270,9 @@ Regex::exec(std::string_view subject, RegexMatches &matches) const } int count = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, matches.get_match_data(), RegexContext::get_instance()->get_match_context()); + + matches.set_size(count); + if (count < 0) { return count; } @@ -247,7 +285,7 @@ Regex::exec(std::string_view subject, RegexMatches &matches) const } //---------------------------------------------------------------------------- -int +int32_t Regex::get_capture_count() { int captures = -1; @@ -279,7 +317,7 @@ DFA::build(const std::string_view pattern, unsigned flags) } //---------------------------------------------------------------------------- -int +int32_t DFA::compile(std::string_view pattern, unsigned flags) { assert(_patterns.empty()); @@ -288,7 +326,7 @@ DFA::compile(std::string_view pattern, unsigned flags) } //---------------------------------------------------------------------------- -int +int32_t DFA::compile(std::string_view *patterns, int npatterns, unsigned flags) { _patterns.reserve(npatterns); // try to pre-allocate. @@ -299,7 +337,7 @@ DFA::compile(std::string_view *patterns, int npatterns, unsigned flags) } //---------------------------------------------------------------------------- -int +int32_t DFA::compile(const char **patterns, int npatterns, unsigned flags) { _patterns.reserve(npatterns); // try to pre-allocate. @@ -310,7 +348,7 @@ DFA::compile(const char **patterns, int npatterns, unsigned flags) } //---------------------------------------------------------------------------- -int +int32_t DFA::match(std::string_view str) const { for (auto spot = _patterns.begin(), limit = _patterns.end(); spot != limit; ++spot) { diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index 4f1a522c224..e2332272f63 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -20,8 +20,8 @@ limitations under the License. */ -#include #include +#include #include "tscore/ink_assert.h" #include "tscore/ink_defs.h" @@ -35,16 +35,64 @@ struct subject_match_t { struct test_t { std::string_view regex; - std::array tests; + std::vector tests; }; -std::array test_data{ - {{{"^foo"}, {{{{"foo"}, true}, {{"bar"}, false}, {{"foobar"}, true}, {{"foobarbaz"}, true}}}}, - {{"foo$"}, {{{{"foo"}, true}, {{"bar"}, false}, {{"foobar"}, false}, {{"foobarbaz"}, false}}}}} +std::vector test_data{ + { + {{R"(^foo)"}, {{{{"foo"}, true}, {{"bar"}, false}, {{"foobar"}, true}, {{"foobarbaz"}, true}}}}, + {{R"(foo$)"}, {{{{"foo"}, true}, {{"bar"}, false}, {{"foobar"}, false}, {{"foobarbaz"}, false}}}}, + // url regular expression + {{R"(^(https?:\/\/)?([\da-z\.-]+)\.([a-z\.]{2,6})([\/\w \.-]*)*\/?$)"}, + {{{{"http://www.example.com"}, true}, + {{"https://www.example.com"}, true}, + {{"http://~example.com"}, false}, + {{"http://www.example.com/foo/bar"}, true}}}}, + // ip address regular expression + {R"(^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$)", + {{{{"1.2.3.4"}, true}, {{"127.0.0.1"}, true}, {{"256.256.256.256"}, false}, {{".1.1.1.1"}, false}}}}, + } +}; + +// test case insensitive test data +std::vector test_data_case_insensitive{ + { + {{R"(^foo)"}, {{{{"FoO"}, true}, {{"bar"}, false}, {{"foObar"}, true}, {{"foobaRbaz"}, true}}}}, + {{R"(foo$)"}, {{{{"foO"}, true}, {{"bar"}, false}, {{"foobar"}, false}, {{"foobarbaz"}, false}}}}, + } +}; + +// test case for anchored flag +std::vector test_data_anchored{ + { + {{R"(foo)"}, {{{{"foo"}, true}, {{"bar"}, false}, {{"foobar"}, true}, {{"foobarbaz"}, true}}}}, + {{R"(bar)"}, {{{{"foo"}, false}, {{"bar"}, true}, {{"foobar"}, false}, {{"foobarbaz"}, false}}}}, + } +}; + +struct submatch_t { + std::string_view subject; + int32_t count; + std::vector submatches; +}; + +struct submatch_test_t { + std::string_view regex; + int capture_count; + std::vector tests; +}; + +std::vector submatch_test_data{ + { + {{R"(^foo)"}, 0, {{{{"foo"}, 1, {{"foo"}}}, {{"bar"}, -1, {}}, {{"foobar"}, 1, {{"foo"}}}, {{"foobarbaz"}, 1, {{"foo"}}}}}}, + {{R"(foo$)"}, 0, {{{{"foo"}, 1, {{"foo"}}}, {{"bar"}, -1, {}}, {{"foobar"}, -1, {}}, {{"foobarbaz"}, -1, {}}}}}, + {{R"(^(foo)(bar))"}, 2, {{{{"foobar"}, 3, {{"foobar", "foo", "bar"}}}, {{"barfoo"}, -1, {}}, {{"foo"}, -1, {}}}}}, + } }; TEST_CASE("Regex", "[libts][Regex]") { + // case sensitive test for (auto &item : test_data) { Regex r; REQUIRE(r.compile(item.regex.data()) == true); @@ -53,4 +101,65 @@ TEST_CASE("Regex", "[libts][Regex]") REQUIRE(r.exec(test.subject.data()) == test.match); } } + + // case insensitive test + for (auto &item : test_data_case_insensitive) { + Regex r; + REQUIRE(r.compile(item.regex.data(), RE_CASE_INSENSITIVE) == true); + + for (auto &test : item.tests) { + REQUIRE(r.exec(test.subject.data()) == test.match); + } + } + + // case anchored test + for (auto &item : test_data_anchored) { + Regex r; + REQUIRE(r.compile(item.regex.data(), RE_ANCHORED) == true); + + for (auto &test : item.tests) { + REQUIRE(r.exec(test.subject.data()) == test.match); + } + } + + // test for invalid regular expression + { + Regex r; + REQUIRE(r.compile(R"((\d+)", RE_CASE_INSENSITIVE) == false); + } + + // test getting submatches with operator[] + for (auto &item : submatch_test_data) { + Regex r; + REQUIRE(r.compile(item.regex.data()) == true); + REQUIRE(r.get_capture_count() == item.capture_count); + + for (auto &test : item.tests) { + RegexMatches matches; + REQUIRE(r.exec(test.subject.data(), matches) == test.count); + REQUIRE(matches.size() == test.count); + + for (int32_t i = 0; i < test.count; i++) { + REQUIRE(matches[i] == test.submatches[i]); + } + } + } + + // test getting submatches with ovector pointer + for (auto &item : submatch_test_data) { + Regex r; + REQUIRE(r.compile(item.regex.data()) == true); + REQUIRE(r.get_capture_count() == item.capture_count); + + for (auto &test : item.tests) { + RegexMatches matches; + REQUIRE(r.exec(test.subject.data(), matches) == test.count); + REQUIRE(matches.size() == test.count); + + size_t *ovector = matches.get_ovector_pointer(); + for (int32_t i = 0; i < test.count; i++) { + REQUIRE(test.submatches[i] == std::string_view{test.subject.data() + ovector[i * 2], ovector[i * 2 + 1] - ovector[i * 2]}); + } + } + } } From 8e3fe56bbe0dea2e3e633d738f0fecb6f4a53eb8 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Wed, 28 Feb 2024 14:07:27 -0800 Subject: [PATCH 11/12] Added more edge case tests Checking against nullptr in conditionals Moved cmake pkgconfig test to top level CMakeLists.txt --- CMakeLists.txt | 2 +- include/tsutil/Regex.h | 4 ---- src/tsutil/CMakeLists.txt | 2 -- src/tsutil/Regex.cc | 18 +++++++++--------- src/tsutil/unit_tests/test_Regex.cc | 29 +++++++++++++++++++++++------ 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 63ec25723a1..a276d827cc5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -253,7 +253,7 @@ if(LibLZMA_FOUND) endif() find_package(PCRE REQUIRED) -find_package(PCRE2 COMPONENTS 8BIT) +pkg_check_modules(PCRE2 REQUIRED IMPORTED_TARGET libpcre2-8) include(CheckOpenSSLIsBoringSSL) include(CheckOpenSSLIsQuictls) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 75a27c142e7..c4ca8feb03c 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -29,11 +29,7 @@ #include #define PCRE2_CODE_UNIT_WIDTH 8 -#if __has_include() -#include -#else #include -#endif /// @brief Match flags for regular expression evaluation. enum REFlags { diff --git a/src/tsutil/CMakeLists.txt b/src/tsutil/CMakeLists.txt index c2538fabfb3..a747431daa7 100644 --- a/src/tsutil/CMakeLists.txt +++ b/src/tsutil/CMakeLists.txt @@ -51,8 +51,6 @@ add_library( Regex.cc ) -pkg_check_modules(PCRE2 REQUIRED IMPORTED_TARGET libpcre2-8) - add_library(ts::tsutil ALIAS tsutil) set_target_properties(tsutil PROPERTIES POSITION_INDEPENDENT_CODE TRUE PUBLIC_HEADER "${TSUTIL_PUBLIC_HEADERS}") target_link_libraries(tsutil PUBLIC libswoc::libswoc yaml-cpp::yaml-cpp PkgConfig::PCRE2) diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index e077e30886a..faea3b8546d 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -57,16 +57,16 @@ class RegexContext } ~RegexContext() { - if (_general_context) { + if (_general_context != nullptr) { pcre2_general_context_free(_general_context); } - if (_compile_context) { + if (_compile_context != nullptr) { pcre2_compile_context_free(_compile_context); } - if (_match_context) { + if (_match_context != nullptr) { pcre2_match_context_free(_match_context); } - if (_jit_stack) { + if (_jit_stack != nullptr) { pcre2_jit_stack_free(_jit_stack); } } @@ -143,7 +143,7 @@ RegexMatches::malloc(size_t size, void *caller) //---------------------------------------------------------------------------- RegexMatches::~RegexMatches() { - if (_match_data) { + if (_match_data != nullptr) { pcre2_match_data_free(_match_data); } } @@ -206,7 +206,7 @@ Regex::Regex(Regex &&that) noexcept //---------------------------------------------------------------------------- Regex::~Regex() { - if (_code) { + if (_code != nullptr) { pcre2_code_free(_code); } } @@ -225,7 +225,7 @@ Regex::compile(std::string_view pattern, uint32_t flags) bool Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, uint32_t flags) { - if (_code) { + if (_code != nullptr) { pcre2_code_free(_code); } PCRE2_SIZE error_offset; @@ -252,7 +252,7 @@ Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, u bool Regex::exec(std::string_view subject) const { - if (!_code) { + if (_code == nullptr) { return false; } RegexMatches matches; @@ -265,7 +265,7 @@ Regex::exec(std::string_view subject) const int32_t Regex::exec(std::string_view subject, RegexMatches &matches) const { - if (!_code) { + if (_code == nullptr) { return 0; } int count = pcre2_match(_code, reinterpret_cast(subject.data()), subject.size(), 0, 0, matches.get_match_data(), diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index e2332272f63..f17d2b17c8e 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -122,12 +122,6 @@ TEST_CASE("Regex", "[libts][Regex]") } } - // test for invalid regular expression - { - Regex r; - REQUIRE(r.compile(R"((\d+)", RE_CASE_INSENSITIVE) == false); - } - // test getting submatches with operator[] for (auto &item : submatch_test_data) { Regex r; @@ -162,4 +156,27 @@ TEST_CASE("Regex", "[libts][Regex]") } } } + + // test for invalid regular expression + { + Regex r; + REQUIRE(r.compile(R"((\d+)", RE_CASE_INSENSITIVE) == false); + } + + // test for not compiling regular expression + { + Regex r; + RegexMatches matches; + REQUIRE(r.exec("foo") == false); + REQUIRE(r.exec("foo", matches) == 0); + } + + // test for recompiling the regular expression + { + Regex r; + REQUIRE(r.compile(R"(foo)") == true); + REQUIRE(r.exec("foo") == true); + REQUIRE(r.compile(R"(bar)") == true); + REQUIRE(r.exec("bar") == true); + } } From 177394133d79ca28b19388d7a4955b27628a1465 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 29 Feb 2024 09:38:31 -0800 Subject: [PATCH 12/12] Added linking with tsutil to tls_bridge plugin --- plugins/experimental/tls_bridge/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/experimental/tls_bridge/CMakeLists.txt b/plugins/experimental/tls_bridge/CMakeLists.txt index e3de8b92498..54305054315 100644 --- a/plugins/experimental/tls_bridge/CMakeLists.txt +++ b/plugins/experimental/tls_bridge/CMakeLists.txt @@ -17,5 +17,5 @@ add_atsplugin(tls_bridge tls_bridge.cc) -target_link_libraries(tls_bridge PRIVATE libswoc::libswoc) +target_link_libraries(tls_bridge PRIVATE ts::tsutil libswoc::libswoc) verify_global_plugin(tls_bridge)