Walk the unpacked tree without following symlinks in the mtime pass (#220)

After extracting a tarball to disk, unpack refreshed every file's mtime
by listing the tree with filelib:wildcard("**"), which descends into
symlinked directories, and file:write_file_info/3, which resolves
symlinks. Packages created by old clients can contain in-tree symlink
cycles (create rejects them today, unpack accepts them), and on such a
package the pass visits every file once per cycle expansion: svx 0.2.0
holds 1,171 files but the wildcard enumerates ~47k paths, turning a
260ms extraction into multiple seconds of mtime updates.

Replace the wildcard with a read_link_info walk that updates regular
files and directories and skips symlinks entirely, computing the
timestamp once.

Author: ericmj

src/hex_tarball.erl

@@ -487,10 +487,7 @@ finish_unpack(#{
             filelib:ensure_dir(filename:join(Output, "*")),
             case unpack_contents(Contents, Output, MaxUncompressedSize) of
                 ok ->
-                    [
-                        try_updating_mtime(filename:join(Output, P))
-                     || P <- filelib:wildcard("**", Output)
-                    ],
+                    update_mtimes(Output),
                     copy_metadata_config(Output, maps:get("metadata.config", Files)),
                     {ok, Result};
                 {error, Reason} ->
@@ -1099,10 +1096,7 @@ unpack_tarball(Source, Output, MaxSize) ->
     filelib:ensure_dir(filename:join(Output, "*")),
     case hex_erl_tar:extract(Source, [{cwd, Output}, compressed, {max_size, MaxSize}]) of
         ok ->
-            [
-                try_updating_mtime(filename:join(Output, Path))
-             || Path <- filelib:wildcard("**", Output)
-            ],
+            update_mtimes(Output),
             ok;
         {error, too_big} ->
             {error, {tarball, {too_big_uncompressed, MaxSize}}};
@@ -1111,11 +1105,30 @@ unpack_tarball(Source, Output, MaxSize) ->
     end.
 
 %% @private
-%% let it silently fail for bad symlinks
-try_updating_mtime(Path) ->
-    Time = calendar:universal_time(),
-    _ = file:write_file_info(Path, #file_info{mtime = Time}, [{time, universal}]),
-    ok.
+%% Skips symlinks: write_file_info/3 follows them, and descending into
+%% symlinked directories loops on link cycles. Failures are ignored.
+update_mtimes(Dir) ->
+    update_mtimes(Dir, calendar:universal_time()).
+
+update_mtimes(Dir, Time) ->
+    case file:list_dir(Dir) of
+        {ok, Names} ->
+            lists:foreach(fun(Name) -> update_mtime(filename:join(Dir, Name), Time) end, Names);
+        {error, _} ->
+            ok
+    end.
+
+update_mtime(Path, Time) ->
+    case file:read_link_info(Path, [{time, universal}]) of
+        {ok, #file_info{type = directory}} ->
+            _ = file:write_file_info(Path, #file_info{mtime = Time}, [{time, universal}]),
+            update_mtimes(Path, Time);
+        {ok, #file_info{type = regular}} ->
+            _ = file:write_file_info(Path, #file_info{mtime = Time}, [{time, universal}]),
+            ok;
+        _ ->
+            ok
+    end.
 
 %% @private
 create_memory_tarball(Files) ->

test/hex_tarball_SUITE.erl

@@ -12,6 +12,7 @@ all() ->
         timestamps_and_permissions_test,
         symlinks_test,
         symlinks_parent_dir_test,
+        symlink_cycle_test,
         unsafe_paths_to_create_test,
         unsupported_file_types_to_create_test,
         memory_test,
@@ -278,6 +279,34 @@ symlinks_parent_dir_test(Config) ->
 
     ok.
 
+%% dir/loop -> .. resolves inside the extraction dir but forms a cycle, so the
+%% post-unpack mtime pass must not traverse symlinked directories.
+symlink_cycle_test(Config) ->
+    BaseDir = ?config(priv_dir, Config),
+    Metadata = #{<<"name">> => <<"cycle">>, <<"version">> => <<"1.0.0">>},
+
+    Dir = filename:join(BaseDir, "cycle_dir"),
+    ok = file:make_dir(Dir),
+    ok = file:write_file(filename:join(Dir, "foo.sh"), <<"foo">>),
+    ok = file:make_symlink("..", filename:join(Dir, "loop")),
+
+    Files = [
+        {"dir/foo.sh", filename:join("cycle_dir", "foo.sh")},
+        {"dir/loop", filename:join("cycle_dir", "loop")}
+    ],
+    CreateConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()),
+
+    {ok, #{tarball := Tarball}} = hex_tarball:create(Metadata, Files, CreateConfig),
+    UnpackDir = filename:join(BaseDir, "symlink_cycle"),
+    {ok, _} = hex_tarball:unpack(Tarball, UnpackDir),
+
+    {ok, #file_info{type = symlink}} =
+        file:read_link_info(filename:join([UnpackDir, "dir", "loop"])),
+    {ok, FooShInfo} = file:read_file_info(filename:join([UnpackDir, "dir", "foo.sh"])),
+    [{{Year, _, _}, _}] = calendar:local_time_to_universal_time_dst(FooShInfo#file_info.mtime),
+    {{Year, _, _}, _} = calendar:local_time(),
+    ok.
+
 unsafe_paths_to_create_test(Config) ->
     BaseDir = ?config(priv_dir, Config),
     Metadata = #{<<"name">> => <<"foo">>, <<"version">> => <<"1.0.0">>},