[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1362: MINIFICPP-1830 Verify regex properties in processors in case it is set to empty

[GitBox <gi...@apache.org>]

fgerlits commented on code in PR #1362:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1362#discussion_r924354272


##########
extensions/civetweb/processors/ListenHTTP.cpp:
##########
@@ -275,10 +277,10 @@ void ListenHTTP::processRequestBuffer(core::ProcessSession *session) {
   logger_->log_debug("ListenHTTP transferred %zu flow files from HTTP request buffer", flow_file_count);
 }
 
-ListenHTTP::Handler::Handler(std::string base_uri, core::ProcessContext *context, std::string &&auth_dn_regex, std::string &&header_as_attrs_regex)
+ListenHTTP::Handler::Handler(std::string base_uri, core::ProcessContext *context, std::string &&auth_dn_regex, std::optional<std::string> &&headers_as_attrs_regex)
     : base_uri_(std::move(base_uri)),
       auth_dn_regex_(std::move(auth_dn_regex)),
-      headers_as_attrs_regex_(std::move(header_as_attrs_regex)),
+      headers_as_attrs_regex_(std::move(headers_as_attrs_regex)),
       process_context_(context) {

Review Comment:
   As far as I can tell, this constructor is only called from `ListenHTTP.cpp:206`, where the last argument is a `string&&`, from which a temporary `optional<string>` parameter is constructed (not moved), from which the `std::optional<utils::Regex> headers_as_attrs_regex_` field is constructed (not moved).  As it is, I don't think it's possible for `headers_as_attrs_regex_` to be `nullopt`.



##########
extensions/standard-processors/processors/GetFile.cpp:
##########
@@ -244,7 +244,7 @@ bool GetFile::fileMatchesRequestCriteria(std::string fullName, std::string name,
     return false;
 
   utils::Regex rgx(request.fileFilter);
-  if (!utils::regexSearch(name, rgx)) {

Review Comment:
   This is a breaking change, as shown by the unit test.  I think changing the default value of `FileFilter` to `.*` (which it really [almost] was, with `regexSearch`) would lessen its impact.



##########
extensions/librdkafka/PublishKafka.cpp:
##########
@@ -184,12 +184,12 @@ class ReadCallback {
     });
   }
 
-  static rd_kafka_headers_unique_ptr make_headers(const core::FlowFile& flow_file, utils::Regex& attribute_name_regex) {
+  static rd_kafka_headers_unique_ptr make_headers(const core::FlowFile& flow_file, std::optional<utils::Regex>& attribute_name_regex) {

Review Comment:
   could this be `const optional<Regex>&`?  (also at line 241)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org