szaszm commented on code in PR #1725:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1725#discussion_r1512874899


##########
libminifi/src/core/json/JsonFlowSerializer.cpp:
##########
@@ -47,14 +47,21 @@ void 
JsonFlowSerializer::encryptSensitiveProperties(rapidjson::Value &property_j
     }
     if (properties.at(name).isSensitive()) {
       auto& value = property.value;
-      const std::string_view value_sv{value.GetString(), 
value.GetStringLength()};
+      const std::string_view value_sv = component_overrides.contains(name) ? 
component_overrides.at(name) : std::string_view{value.GetString(), 
value.GetStringLength()};
       const std::string encrypted_value = 
utils::crypto::property_encryption::encrypt(value_sv, encryption_provider);
       value.SetString(encrypted_value.c_str(), encrypted_value.size(), alloc);
     }
+    component_overrides.erase(name);
+  }
+
+  for (const auto& [name, value] : component_overrides) {
+    const std::string encrypted_value = 
utils::crypto::property_encryption::encrypt(value, encryption_provider);
+    property_jsons.AddMember(rapidjson::Value(name, alloc), 
rapidjson::Value(encrypted_value, alloc), alloc);

Review Comment:
   Overrides as a name is fine for me, but their expected format needs to be 
clearly defined.
   I think we should standardize whether the values in the overrides are 
already encrypted (if sensitive) or not encrypted. I'm not sure how we 
currently decide about encryption based on this control flow.
   



-- 
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-unsubscr...@nifi.apache.org

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

Reply via email to