You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2018/04/04 17:18:27 UTC

[trafficserver] 01/03: Fixes various issues with brotli compression

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 33ae4b20069b0ef564fc26e2c92abb8e2ae3c186
Author: Randall Meyer <ra...@yahoo.com>
AuthorDate: Fri Mar 16 16:50:47 2018 -0700

    Fixes various issues with brotli compression
    
    brotli API needs operations to be repeated until done.
    flush needs to occur after block is processed.
    put all brotli code paths under HAVE_BROTLI_ENCODE_H.
    merge redundant code paths into a single function.
    
    Addresses issue #2965 and #2947
    
    (cherry picked from commit 35a512cba6a13de9309f2869cf36d134fed3bb35)
---
 plugins/gzip/gzip.cc | 141 +++++++++++++++++++++++++--------------------------
 1 file changed, 68 insertions(+), 73 deletions(-)

diff --git a/plugins/gzip/gzip.cc b/plugins/gzip/gzip.cc
index 1e34ab1..cc071a2 100644
--- a/plugins/gzip/gzip.cc
+++ b/plugins/gzip/gzip.cc
@@ -335,18 +335,19 @@ gzip_transform_one(Data *data, const char *upstream_buffer, int64_t upstream_len
   }
 }
 
-static void
-brotli_transform_one(Data *data, const char *upstream_buffer, int64_t upstream_length)
-{
 #if HAVE_BROTLI_ENCODE_H
+static bool
+brotli_compress_operation(Data *data, const char *upstream_buffer, int64_t upstream_length, BrotliEncoderOperation op)
+{
   TSIOBufferBlock downstream_blkp;
   char *downstream_buffer;
   int64_t downstream_length;
-  int err;
 
+  data->bstrm.next_in  = (uint8_t *)upstream_buffer;
   data->bstrm.avail_in = upstream_length;
 
-  while (data->bstrm.avail_in > 0) {
+  bool ok = true;
+  while (ok) {
     downstream_blkp   = TSIOBufferStart(data->downstream_buffer);
     downstream_buffer = TSIOBufferBlockWriteStart(downstream_blkp, &downstream_length);
 
@@ -354,37 +355,49 @@ brotli_transform_one(Data *data, const char *upstream_buffer, int64_t upstream_l
     data->bstrm.avail_out = downstream_length;
     data->bstrm.total_out = 0;
 
-    data->bstrm.next_in = (uint8_t *)upstream_buffer;
-    if (!data->hc->flush()) {
-      err = BrotliEncoderCompressStream(data->bstrm.br, BROTLI_OPERATION_PROCESS, &data->bstrm.avail_in,
-                                        (const uint8_t **)&data->bstrm.next_in, &data->bstrm.avail_out, &data->bstrm.next_out,
-                                        &data->bstrm.total_out);
-    } else {
-      err = BrotliEncoderCompressStream(data->bstrm.br, BROTLI_OPERATION_FLUSH, &data->bstrm.avail_in,
-                                        (const uint8_t **)&data->bstrm.next_in, &data->bstrm.avail_out, &data->bstrm.next_out,
-                                        &data->bstrm.total_out);
-    }
+    ok =
+      !!BrotliEncoderCompressStream(data->bstrm.br, op, &data->bstrm.avail_in, &const_cast<const uint8_t *&>(data->bstrm.next_in),
+                                    &data->bstrm.avail_out, &data->bstrm.next_out, &data->bstrm.total_out);
 
-    if (err != BROTLI_TRUE) {
-      warning("BrotliEncoderCompressStream() call failed: %d", err);
+    if (!ok) {
+      error("BrotliEncoderCompressStream(%d) call failed", op);
+      return ok;
     }
 
-    if (downstream_length > (int64_t)data->bstrm.avail_out) {
-      TSIOBufferProduce(data->downstream_buffer, downstream_length - data->bstrm.avail_out);
-      data->downstream_length += (downstream_length - data->bstrm.avail_out);
+    TSIOBufferProduce(data->downstream_buffer, downstream_length - data->bstrm.avail_out);
+    data->downstream_length += (downstream_length - data->bstrm.avail_out);
+    if (data->bstrm.avail_in || BrotliEncoderHasMoreOutput(data->bstrm.br)) {
+      continue;
     }
 
-    if (data->bstrm.avail_out > 0) {
-      if (data->bstrm.avail_in != 0) {
-        error("brotli-transform: ERROR: brotli avail_in is (%lu): should be 0", data->bstrm.avail_in);
-      }
-    }
+    break;
   }
+
+  return ok;
+}
+
+static void
+brotli_transform_one(Data *data, const char *upstream_buffer, int64_t upstream_length)
+{
+  bool ok = brotli_compress_operation(data, upstream_buffer, upstream_length, BROTLI_OPERATION_PROCESS);
+  if (!ok) {
+    error("BrotliEncoderCompressStream(PROCESS) call failed");
+    return;
+  }
+
   data->bstrm.total_in += upstream_length;
-#else
-  error("brotli-transform: ERROR: compile with brotli support");
-#endif
+
+  if (!data->hc->flush()) {
+    return;
+  }
+
+  ok = brotli_compress_operation(data, nullptr, 0, BROTLI_OPERATION_FLUSH);
+  if (!ok) {
+    error("BrotliEncoderCompressStream(FLUSH) call failed");
+    return;
+  }
 }
+#endif
 
 static void
 compress_transform_one(Data *data, TSIOBufferReader upstream_reader, int amount)
@@ -409,10 +422,13 @@ compress_transform_one(Data *data, TSIOBufferReader upstream_reader, int amount)
       upstream_length = amount;
     }
 
+#if HAVE_BROTLI_ENCODE_H
     if (data->compression_type & COMPRESSION_TYPE_BROTLI && (data->compression_algorithms & ALGORITHM_BROTLI)) {
       brotli_transform_one(data, upstream_buffer, upstream_length);
-    } else if ((data->compression_type & (COMPRESSION_TYPE_GZIP | COMPRESSION_TYPE_DEFLATE)) &&
-               (data->compression_algorithms & (ALGORITHM_GZIP | ALGORITHM_DEFLATE))) {
+    } else
+#endif
+      if ((data->compression_type & (COMPRESSION_TYPE_GZIP | COMPRESSION_TYPE_DEFLATE)) &&
+          (data->compression_algorithms & (ALGORITHM_GZIP | ALGORITHM_DEFLATE))) {
       gzip_transform_one(data, upstream_buffer, upstream_length);
     } else {
       warning("No compression supported. Shoudn't come here.");
@@ -466,67 +482,46 @@ gzip_transform_finish(Data *data)
   }
 }
 
+#if HAVE_BROTLI_ENCODE_H
 static void
 brotli_transform_finish(Data *data)
 {
-#if HAVE_BROTLI_ENCODE_H
-  if (data->state == transform_state_output) {
-    TSIOBufferBlock downstream_blkp;
-    char *downstream_buffer;
-    int64_t downstream_length;
-    int err;
-
-    data->state = transform_state_finished;
-
-    for (;;) {
-      downstream_blkp = TSIOBufferStart(data->downstream_buffer);
-
-      downstream_buffer     = TSIOBufferBlockWriteStart(downstream_blkp, &downstream_length);
-      data->bstrm.next_out  = (unsigned char *)downstream_buffer;
-      data->bstrm.avail_out = downstream_length;
-
-      err = BrotliEncoderCompressStream(data->bstrm.br, BROTLI_OPERATION_FINISH, &data->bstrm.avail_in,
-                                        (const uint8_t **)&data->bstrm.next_in, &data->bstrm.avail_out, &data->bstrm.next_out,
-                                        &data->bstrm.total_out);
-
-      if (downstream_length > (int64_t)data->bstrm.avail_out) {
-        TSIOBufferProduce(data->downstream_buffer, downstream_length - data->bstrm.avail_out);
-        data->downstream_length += (downstream_length - data->bstrm.avail_out);
-      }
-      if (!BrotliEncoderIsFinished(data->bstrm.br)) {
-        continue;
-      }
+  if (data->state != transform_state_output) {
+    return;
+  }
 
-      if (err != BROTLI_TRUE) { /* some more data to encode */
-        warning("brotli_transform: BrotliEncoderCompressStream should return BROTLI_TRUE");
-      }
+  data->state = transform_state_finished;
 
-      break;
-    }
+  bool ok = brotli_compress_operation(data, nullptr, 0, BROTLI_OPERATION_FINISH);
+  if (!ok) {
+    error("BrotliEncoderCompressStream(PROCESS) call failed");
+    return;
+  }
 
-    if (data->downstream_length != (int64_t)(data->bstrm.total_out)) {
-      error("brotli-transform: ERROR: output lengths don't match (%d, %ld)", data->downstream_length, data->bstrm.total_out);
-    }
-    debug("brotli-transform: Finished brotli");
-    gzip_log_ratio(data->bstrm.total_in, data->downstream_length);
+  if (data->downstream_length != (int64_t)(data->bstrm.total_out)) {
+    error("brotli-transform: output lengths don't match (%d, %ld)", data->downstream_length, data->bstrm.total_out);
   }
-#else
-  error("brotli-transform: compile with brotli support");
-#endif
+
+  debug("brotli-transform: Finished brotli");
+  gzip_log_ratio(data->bstrm.total_in, data->downstream_length);
 }
+#endif
 
 static void
 compress_transform_finish(Data *data)
 {
+#if HAVE_BROTLI_ENCODE_H
   if (data->compression_type & COMPRESSION_TYPE_BROTLI && data->compression_algorithms & ALGORITHM_BROTLI) {
     brotli_transform_finish(data);
     debug("brotli-transform: Brotli compression finish.");
-  } else if ((data->compression_type & (COMPRESSION_TYPE_GZIP | COMPRESSION_TYPE_DEFLATE)) &&
-             (data->compression_algorithms & (ALGORITHM_GZIP | ALGORITHM_DEFLATE))) {
+  } else
+#endif
+    if ((data->compression_type & (COMPRESSION_TYPE_GZIP | COMPRESSION_TYPE_DEFLATE)) &&
+        (data->compression_algorithms & (ALGORITHM_GZIP | ALGORITHM_DEFLATE))) {
     gzip_transform_finish(data);
     debug("gzip-transform: Gzip compression finish.");
   } else {
-    warning("No Compression matched, shouldn't come here.");
+    error("No Compression matched, shouldn't come here.");
   }
 }
 

-- 
To stop receiving notification emails like this one, please contact
zwoop@apache.org.