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


##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -124,10 +133,30 @@ C2Payload RESTSender::sendPayload(const std::string url, 
const Direction directi
     } else {
       auto data_input = std::make_unique<utils::ByteInputCallback>();
       auto data_cb = std::make_unique<utils::HTTPUploadCallback>();
-      data_input->write(data.value_or(""));
+      if (data && req_encoding_ == RequestEncoding::Gzip) {
+        io::BufferStream compressed_payload;
+        bool compression_success = [&] {
+          io::ZlibCompressStream 
compressor(gsl::make_not_null(&compressed_payload), 
io::ZlibCompressionFormat::GZIP, Z_BEST_COMPRESSION);
+          auto ret = compressor.write(gsl::span<const 
char>(data.value()).as_span<const std::byte>());
+          if (ret != data->length()) {
+            return false;

Review Comment:
   we don't need `compressor.close()` in this case?



##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -52,6 +49,18 @@ void 
RESTSender::initialize(core::controller::ControllerServiceProvider* control
         ssl_context_service_ = 
std::static_pointer_cast<minifi::controllers::SSLContextService>(service);
       }
     }
+    if (auto req_encoding_str = 
configure->get(Configuration::nifi_c2_rest_request_encoding)) {
+      if (auto req_encoding = 
RequestEncoding::parse(req_encoding_str->c_str(), RequestEncoding{})) {

Review Comment:
   it might be more user-friendly to do this in a case-insensitive way



##########
conf/minifi.properties:
##########
@@ -102,11 +102,16 @@ 
nifi.c2.root.class.definitions.metrics.metrics.processorMetrics.classes=GetFileM
 ## HeartbeatLogger logs the heartbeats on TRACE for debugging.
 #nifi.c2.agent.heartbeat.reporter.classes=HeartbeatLogger
 
+# specify encoding strategy for c2 requests (gzip, none)
+#nifi.c2.rest.request.encoding=none
+
 ## enable the controller socket provider on port 9998
 ## off by default. C2 must be enabled to support these
 #controller.socket.host=localhost
 #controller.socket.port=9998
 
+## specify the destination of c2 directed assets
+#nifi.asset.directory=${MINIFI_HOME}/asset

Review Comment:
   Should the two `asset.directory`-related changes be in this PR?  They don't 
seem to be related to C2 request compression.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to