Merge pull request #1358 from bjorng/bjorn/stdlib/fix-zip-bugs/OTP-14189

zip: Fix bugs ERL-348 and ERL-349

OTP-14246

2 files changed, 55 insertions, 43 deletions

diff --git a/lib/stdlib/src/zip.erl b/lib/stdlib/src/zip.erl
index 340cc21390..fadf96146e 100644
--- a/lib/stdlib/src/zip.erl
+++ b/lib/stdlib/src/zip.erl
@@ -179,19 +179,6 @@
 			 external_attr,
 			 local_header_offset}).
 
-%% Unix extra fields (not yet supported)
--define(UNIX_EXTRA_FIELD_TAG, 16#000d).
--record(unix_extra_field, {atime,
-			   mtime,
-			   uid,
-			   gid}).
-
-%% extended timestamps (not yet supported)
--define(EXTENDED_TIMESTAMP_TAG, 16#5455).
-%% -record(extended_timestamp, {mtime,
-%% 			     atime,
-%% 			     ctime}).
-
 -define(END_OF_CENTRAL_DIR_MAGIC, 16#06054b50).
 -define(END_OF_CENTRAL_DIR_SZ, (4+2+2+2+2+4+4+2)).
 
@@ -381,9 +368,12 @@ do_unzip(F, Options) ->
     {Info, In1} = get_central_dir(In0, RawIterator, Input),
     %% get rid of zip-comment
     Z = zlib:open(),
-    Files = get_z_files(Info, Z, In1, Opts, []),
-    zlib:close(Z),
-    Input(close, In1),
+    Files = try
+                get_z_files(Info, Z, In1, Opts, [])
+            after
+                zlib:close(Z),
+                Input(close, In1)
+            end,
     {ok, Files}.
 
 %% Iterate over all files in a zip archive
@@ -460,11 +450,20 @@ do_zip(F, Files, Options) ->
     #zip_opts{output = Output, open_opts = OpO} = Opts,
     Out0 = Output({open, F, OpO}, []),
     Z = zlib:open(),
-    {Out1, LHS, Pos} = put_z_files(Files, Z, Out0, 0, Opts, []),
-    zlib:close(Z),
-    Out2 = put_central_dir(LHS, Pos, Out1, Opts),
-    Out3 = Output({close, F}, Out2),
-    {ok, Out3}.
+    try
+        {Out1, LHS, Pos} = put_z_files(Files, Z, Out0, 0, Opts, []),
+        zlib:close(Z),
+        Out2 = put_central_dir(LHS, Pos, Out1, Opts),
+        Out3 = Output({close, F}, Out2),
+        {ok, Out3}
+    catch
+        C:R ->
+            Stk = erlang:get_stacktrace(),
+            zlib:close(Z),
+            Output({close, F}, Out0),
+            erlang:raise(C, R, Stk)
+    end.
+
 
 %% List zip directory contents
 %%
@@ -1379,12 +1378,7 @@ cd_file_header_to_file_info(FileName,
 		    gid = 0},
     add_extra_info(FI, ExtraField).
 
-%% add extra info to file (some day when we implement it)
-add_extra_info(FI, <<?EXTENDED_TIMESTAMP_TAG:16/little, _Rest/binary>>) ->
-    FI;     % not yet supported, some other day...
-add_extra_info(FI, <<?UNIX_EXTRA_FIELD_TAG:16/little, Rest/binary>>) ->
-    _UnixExtra = unix_extra_field_and_var_from_bin(Rest),
-    FI;     % not yet supported, and not widely used
+%% Currently, we ignore all the extra fields.
 add_extra_info(FI, _) ->
     FI.
 
@@ -1572,20 +1566,6 @@ dos_date_time_from_datetime({{Year, Month, Day}, {Hour, Min, Sec}}) ->
     <<DosDate:16>> = <<YearFrom1980:7, Month:4, Day:5>>,
     {DosDate, DosTime}.
 
-unix_extra_field_and_var_from_bin(<<TSize:16/little,
-				   ATime:32/little,
-				   MTime:32/little,
-				   UID:16/little,
-				   GID:16/little,
-				   Var:TSize/binary>>) ->
-    {#unix_extra_field{atime = ATime,
-		       mtime = MTime,
-		       uid = UID,
-		       gid = GID},
-     Var};
-unix_extra_field_and_var_from_bin(_) ->
-    throw(bad_unix_extra_field).
-
 %% A pwrite-like function for iolists (used by memory-option)
 
 pwrite_binary(B, Pos, Bin) when byte_size(B) =:= Pos ->
diff --git a/lib/stdlib/test/zip_SUITE.erl b/lib/stdlib/test/zip_SUITE.erl
index 7d90795c9e..f0feda217a 100644
--- a/lib/stdlib/test/zip_SUITE.erl
+++ b/lib/stdlib/test/zip_SUITE.erl
@@ -27,7 +27,7 @@
          openzip_api/1, zip_api/1, open_leak/1, unzip_jar/1,
 	 unzip_traversal_exploit/1,
          compress_control/1,
-	 foldl/1]).
+	 foldl/1,fd_leak/1]).
 
 -include_lib("common_test/include/ct.hrl").
 -include_lib("kernel/include/file.hrl").
@@ -40,7 +40,7 @@ all() ->
      unzip_to_binary, zip_to_binary, unzip_options,
      zip_options, list_dir_options, aliases, openzip_api,
      zip_api, open_leak, unzip_jar, compress_control, foldl,
-     unzip_traversal_exploit].
+     unzip_traversal_exploit,fd_leak].
 
 groups() -> 
     [].
@@ -882,3 +882,35 @@ foldl(Config) ->
     {error, enoent} = zip:foldl(ZipFun, [], File),
 
     ok.
+
+fd_leak(Config) ->
+    ok = file:set_cwd(proplists:get_value(priv_dir, Config)),
+    DataDir = proplists:get_value(data_dir, Config),
+    Name = filename:join(DataDir, "bad_file_header.zip"),
+    BadExtract = fun() ->
+                         {error,bad_file_header} = zip:extract(Name),
+                         ok
+                 end,
+    do_fd_leak(BadExtract, 1),
+
+    BadCreate = fun() ->
+                        {error,enoent} = zip:zip("failed.zip",
+                                                 ["none"]),
+                        ok
+                end,
+    do_fd_leak(BadCreate, 1),
+
+    ok.
+
+do_fd_leak(_Bad, 10000) ->
+    ok;
+do_fd_leak(Bad, N) ->
+    try Bad() of
+        ok ->
+            do_fd_leak(Bad, N + 1)
+    catch
+        C:R ->
+            Stk = erlang:get_stacktrace(),
+            io:format("Bad error after ~p attempts\n", [N]),
+            erlang:raise(C, R, Stk)
+    end.