bryancall commented on code in PR #12201:
URL: https://github.com/apache/trafficserver/pull/12201#discussion_r2482539158


##########
plugins/compress/configuration.cc:
##########
@@ -208,7 +218,7 @@ 
HostConfiguration::add_compression_algorithms(swoc::TextView line)
     } else if (token == "deflate") {
       compression_algorithms_ |= ALGORITHM_DEFLATE;
     } else {
-      error("Unknown compression type. Supported compression-algorithms 
<br,gzip,deflate>.");
+      error("Unknown compression type. Supported compression-algorithms 
<zstd,br,gzip,deflate>.");

Review Comment:
   Looks like this error should be conditional based on how the plugin was 
compiled



##########
plugins/compress/compress.cc:
##########
@@ -467,6 +504,144 @@ brotli_transform_one(Data *data, const char 
*upstream_buffer, int64_t upstream_l
 }
 #endif
 
+#if HAVE_ZSTD_H
+static bool
+zstd_compress_operation(Data *data, const char *upstream_buffer, int64_t 
upstream_length, ZSTD_EndDirective mode)
+{
+  TSIOBufferBlock downstream_blkp;
+  int64_t         downstream_length;
+
+  ZSTD_inBuffer input = {upstream_buffer, 
static_cast<size_t>(upstream_length), 0};
+
+  for (;;) {
+    downstream_blkp         = TSIOBufferStart(data->downstream_buffer);
+    char *downstream_buffer = TSIOBufferBlockWriteStart(downstream_blkp, 
&downstream_length);
+
+    ZSTD_outBuffer output = {downstream_buffer, 
static_cast<size_t>(downstream_length), 0};
+
+    size_t result = ZSTD_compressStream2(data->zstrm_zstd.cctx, &output, 
&input, mode);
+
+    if (ZSTD_isError(result)) {
+      error("Zstd compression failed (%d): %s", mode, 
ZSTD_getErrorName(result));
+      return false;
+    }
+
+    if (output.pos > 0) {
+      TSIOBufferProduce(data->downstream_buffer, output.pos);
+      data->downstream_length    += output.pos;
+      data->zstrm_zstd.total_out += output.pos;
+    }
+
+    // Check completion conditions based on mode
+    if (mode == ZSTD_e_continue) {
+      // For continue mode, stop when all input is consumed
+      if (input.pos >= input.size) {
+        break;
+      }
+      // If we have output space but no more input was consumed, break to 
avoid infinite loop
+      if (output.pos == 0 && input.pos < input.size) {
+        error("zstd-transform: no progress made in compression");
+        return false;
+      }
+    } else if (mode == ZSTD_e_flush) {
+      // For flush mode, stop when flush is complete (result == 0)
+      if (result == 0) {
+        break;
+      }
+    }
+  }
+
+  return true;
+}
+
+static void
+zstd_transform_init(Data *data)

Review Comment:
   How do we know if the context is partially initialized?  Seems like we 
should return a bool and check the return value.



##########
contrib/docker/ubuntu/noble/Dockerfile:
##########
@@ -48,6 +48,9 @@ RUN apt update \
     libpcre3-dev \
     hwloc \
     libbrotli-dev \
+    libzstd-dev \
+    luajit \
+    luajit \

Review Comment:
   Should only be listed 1 time (luajit)



##########
plugins/compress/compress.cc:
##########
@@ -467,6 +504,144 @@ brotli_transform_one(Data *data, const char 
*upstream_buffer, int64_t upstream_l
 }
 #endif
 
+#if HAVE_ZSTD_H
+static bool
+zstd_compress_operation(Data *data, const char *upstream_buffer, int64_t 
upstream_length, ZSTD_EndDirective mode)
+{
+  TSIOBufferBlock downstream_blkp;
+  int64_t         downstream_length;
+
+  ZSTD_inBuffer input = {upstream_buffer, 
static_cast<size_t>(upstream_length), 0};

Review Comment:
   No check to see if upstream_length is positive, can cause int overflow issue.



##########
plugins/compress/compress.cc:
##########
@@ -467,6 +504,144 @@ brotli_transform_one(Data *data, const char 
*upstream_buffer, int64_t upstream_l
 }
 #endif
 
+#if HAVE_ZSTD_H
+static bool
+zstd_compress_operation(Data *data, const char *upstream_buffer, int64_t 
upstream_length, ZSTD_EndDirective mode)
+{
+  TSIOBufferBlock downstream_blkp;
+  int64_t         downstream_length;
+
+  ZSTD_inBuffer input = {upstream_buffer, 
static_cast<size_t>(upstream_length), 0};
+
+  for (;;) {
+    downstream_blkp         = TSIOBufferStart(data->downstream_buffer);
+    char *downstream_buffer = TSIOBufferBlockWriteStart(downstream_blkp, 
&downstream_length);
+
+    ZSTD_outBuffer output = {downstream_buffer, 
static_cast<size_t>(downstream_length), 0};

Review Comment:
   downstream_length can be an int overflow here too.



-- 
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