Merge branch 'john/erts/zlib-1.2.11-fixes/OTP-14751' into maint-20

* john/erts/zlib-1.2.11-fixes/OTP-14751:
  Avoid WindowBits=8 as per the manual
  Fix deflateParams on zlib 1.2.11

4 files changed, 68 insertions, 18 deletions

diff --git a/erts/emulator/nifs/common/zlib_nif.c b/erts/emulator/nifs/common/zlib_nif.c
index fa29b4fb71..9565a5a059 100644
--- a/erts/emulator/nifs/common/zlib_nif.c
+++ b/erts/emulator/nifs/common/zlib_nif.c
@@ -717,7 +717,9 @@ static ERL_NIF_TERM zlib_deflateEnd(ErlNifEnv *env, int argc, const ERL_NIF_TERM
 
 static ERL_NIF_TERM zlib_deflateParams(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) {
     zlib_data_t *d;
+
     int res, level, strategy;
+    Bytef dummy_buffer;
 
     if(argc != 3 || !get_zlib_data(env, argv[0], &d)
                  || !enif_get_int(env, argv[1], &level)
@@ -729,12 +731,27 @@ static ERL_NIF_TERM zlib_deflateParams(ErlNifEnv *env, int argc, const ERL_NIF_T
         return enif_raise_exception(env, am_not_initialized);
     }
 
-    /* deflateParams will flush everything currently in the stream, corrupting
-     * the heap unless it's empty. We therefore pretend to have a full output
-     * buffer, forcing a Z_BUF_ERROR if there's anything left to be flushed. */
-    d->s.avail_out = 0;
+    /* This is a bit of a hack; deflateParams flushes with Z_BLOCK which won't
+     * stop at a byte boundary, so we can't split this operation up, and we
+     * can't allocate a buffer large enough to fit it in one go since we have
+     * to support zlib versions that lack deflatePending.
+     *
+     * We therefore flush everything prior to this call to ensure that we are
+     * stopped on a byte boundary and have no pending data. We then hand it a
+     * dummy buffer to detect when this assumption doesn't hold (Hopefully
+     * never), and to smooth over an issue with zlib 1.2.11 which always
+     * returns Z_BUF_ERROR when d->s.avail_out is 0, regardless of whether
+     * there's any pending data or not. */
+
+    d->s.next_out = &dummy_buffer;
+    d->s.avail_out = 1;
+
     res = deflateParams(&d->s, level, strategy);
 
+    if(d->s.avail_out == 0) {
+        return zlib_return(env, Z_STREAM_ERROR);
+    }
+
     return zlib_return(env, res);
 }
 
diff --git a/erts/preloaded/ebin/zlib.beam b/erts/preloaded/ebin/zlib.beam
index f388bc723a..4ad5f37434 100644
--- a/erts/preloaded/ebin/zlib.beam
+++ b/erts/preloaded/ebin/zlib.beam
diff --git a/erts/preloaded/src/zlib.erl b/erts/preloaded/src/zlib.erl
index 03c9ae38a1..a4ef42204d 100644
--- a/erts/preloaded/src/zlib.erl
+++ b/erts/preloaded/src/zlib.erl
@@ -188,14 +188,13 @@ deflateReset_nif(_Z) ->
 deflateParams(Z, Level0, Strategy0) ->
     Level = arg_level(Level0),
     Strategy = arg_strategy(Strategy0),
+    Progress = deflate(Z, <<>>, sync),
     case deflateParams_nif(Z, Level, Strategy) of
-        buf_error ->
-            %% We had data left in the pipe; flush everything and stash it away
-            %% for the next deflate call before trying again.
-            Output = deflate(Z, <<>>, full),
-            save_progress(Z, deflate, Output),
-            deflateParams_nif(Z, Level, Strategy);
-        Any -> Any
+        ok ->
+            save_progress(Z, deflate, Progress),
+            ok;
+        Other ->
+            Other
     end.
 deflateParams_nif(_Z, _Level, _Strategy) ->
     erlang:nif_error(undef).
diff --git a/lib/kernel/test/zlib_SUITE.erl b/lib/kernel/test/zlib_SUITE.erl
index 1afcd155b3..6d4e21b8cf 100644
--- a/lib/kernel/test/zlib_SUITE.erl
+++ b/lib/kernel/test/zlib_SUITE.erl
@@ -166,7 +166,7 @@ api_deflateInit(Config) when is_list(Config) ->
 			  ?m(ok, zlib:deflateInit(Z12,default,deflated,-Wbits,8,default)),
 			  ?m(ok,zlib:close(Z11)),
 			  ?m(ok,zlib:close(Z12))
-		  end, lists:seq(8, 15)),
+		  end, lists:seq(9, 15)),
 
     lists:foreach(fun(MemLevel) ->
 			  Z = zlib:open(),
@@ -213,12 +213,46 @@ api_deflateReset(Config) when is_list(Config) ->
 
 %% Test deflateParams.
 api_deflateParams(Config) when is_list(Config) ->
+    Levels = [none, default, best_speed, best_compression] ++ lists:seq(0, 9),
+    Strategies = [filtered, huffman_only, rle, default],
+
     Z1 = zlib:open(),
     ?m(ok, zlib:deflateInit(Z1, default)),
-    ?m(L when is_list(L), zlib:deflate(Z1, <<1,1,1,1,1,1,1,1,1>>, none)),
-    ?m(ok, zlib:deflateParams(Z1, best_compression, huffman_only)),
-    ?m(L when is_list(L), zlib:deflate(Z1, <<1,1,1,1,1,1,1,1,1>>, sync)),
-    ?m(ok, zlib:close(Z1)).
+
+    ApiTest =
+        fun(Level, Strategy) ->
+            ?m(ok, zlib:deflateParams(Z1, Level, Strategy)),
+            ?m(ok, zlib:deflateReset(Z1))
+        end,
+
+    [ ApiTest(Level, Strategy) || Level <- Levels, Strategy <- Strategies ],
+
+    ?m(ok, zlib:close(Z1)),
+
+    FlushTest =
+        fun FlushTest(Size, Level, Strategy) ->
+            Z = zlib:open(),
+            ok = zlib:deflateInit(Z, default),
+            Data = gen_determ_rand_bytes(Size),
+            case zlib:deflate(Z, Data, none) of
+                [<<120, 156>>] ->
+                    %% All data is present in the internal zlib state, and will
+                    %% be flushed on deflateParams.
+
+                    ok = zlib:deflateParams(Z, Level, Strategy),
+                    Compressed = [<<120, 156>>, zlib:deflate(Z, <<>>, finish)],
+                    Data = zlib:uncompress(Compressed),
+                    zlib:close(Z),
+
+                    FlushTest(Size + (1 bsl 10), Level, Strategy);
+                _Other ->
+                    ok
+            end
+        end,
+
+    [ FlushTest(1, Level, Strategy) || Level <- Levels, Strategy <- Strategies ],
+
+    ok.
 
 %% Test deflate.
 api_deflate(Config) when is_list(Config) ->
@@ -762,13 +796,13 @@ zip_usage({run,ZIP,ORIG}) ->
 
     ?m(ok, zlib:deflateInit(Z, default, deflated, -15, 8, default)),
     C2 = zlib:deflate(Z, ORIG, finish),
-    ?m(true, C1 == list_to_binary(C2)),
+    ?m(ORIG, zlib:unzip(C2)),
     ?m(ok, zlib:deflateEnd(Z)),
 
     ?m(ok, zlib:deflateInit(Z, none, deflated, -15, 8, filtered)),
     ?m(ok, zlib:deflateParams(Z, default, default)),
     C3 = zlib:deflate(Z, ORIG, finish),
-    ?m(true, C1 == list_to_binary(C3)),
+    ?m(ORIG, zlib:unzip(C3)),
     ?m(ok, zlib:deflateEnd(Z)),
 
     ok = zlib:close(Z),