Explorar o código

fix(ft): ensure temp filenames are under 255 bytes long

Andrew Mayorov %!s(int64=2) %!d(string=hai) anos
pai
achega
69b98c1830

+ 23 - 0
apps/emqx_ft/src/emqx_ft_fs_util.erl

@@ -29,6 +29,8 @@
 
 -export([fold/4]).
 
+-export([mk_temp_filename/1]).
+
 -type foldfun(Acc) ::
     fun(
         (
@@ -178,3 +180,24 @@ fold(FoldFun, Acc, It) ->
         none ->
             Acc
     end.
+
+-spec mk_temp_filename(file:filename()) ->
+    file:filename().
+mk_temp_filename(Filename) ->
+    % NOTE
+    % Using only the first 200 characters of the filename to avoid making filenames
+    % exceeding 255 bytes in UTF-8. It's actually too conservative, `Suffix` can be
+    % at most 16 bytes.
+    Unique = erlang:unique_integer([positive]),
+    Suffix = binary:encode_hex(<<Unique:64>>),
+    mk_filename([string:slice(Filename, 0, 200), ".", Suffix]).
+
+mk_filename(Comps) ->
+    lists:append(lists:map(fun mk_filename_component/1, Comps)).
+
+mk_filename_component(A) when is_atom(A) ->
+    atom_to_list(A);
+mk_filename_component(B) when is_binary(B) ->
+    unicode:characters_to_list(B);
+mk_filename_component(S) when is_list(S) ->
+    S.

+ 2 - 2
apps/emqx_ft/src/emqx_ft_schema.erl

@@ -145,7 +145,7 @@ fields(local_storage_segments) ->
     [
         {root,
             mk(
-                binary(),
+                string(),
                 #{
                     desc => ?DESC("local_storage_segments_root"),
                     required => false
@@ -182,7 +182,7 @@ fields(local_storage_exporter) ->
     [
         {root,
             mk(
-                binary(),
+                string(),
                 #{
                     desc => ?DESC("local_storage_exporter_root"),
                     required => false

+ 1 - 2
apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl

@@ -452,8 +452,7 @@ mk_manifest_filename(Filename) when is_binary(Filename) ->
     <<Filename/binary, ?MANIFEST>>.
 
 mk_temp_absfilepath(Options, Transfer, Filename) ->
-    Unique = erlang:unique_integer([positive]),
-    TempFilename = integer_to_list(Unique) ++ "." ++ Filename,
+    TempFilename = emqx_ft_fs_util:mk_temp_filename(Filename),
     filename:join(mk_absdir(Options, Transfer, temporary), TempFilename).
 
 mk_absdir(Options, _Transfer, temporary) ->

+ 2 - 10
apps/emqx_ft/src/emqx_ft_storage_fs.erl

@@ -445,16 +445,8 @@ write_file_atomic(Storage, Transfer, Filepath, Content) when is_binary(Content)
     end.
 
 mk_temp_filepath(Storage, Transfer, Filename) ->
-    Unique = erlang:unique_integer([positive]),
-    filename:join(get_subdir(Storage, Transfer, temporary), mk_filename([Unique, ".", Filename])).
-
-mk_filename(Comps) ->
-    lists:append(lists:map(fun mk_filename_component/1, Comps)).
-
-mk_filename_component(I) when is_integer(I) -> integer_to_list(I);
-mk_filename_component(A) when is_atom(A) -> atom_to_list(A);
-mk_filename_component(B) when is_binary(B) -> unicode:characters_to_list(B);
-mk_filename_component(S) when is_list(S) -> S.
+    TempFilename = emqx_ft_fs_util:mk_temp_filename(Filename),
+    filename:join(get_subdir(Storage, Transfer, temporary), TempFilename).
 
 write_contents(Filepath, Content) ->
     file:write_file(Filepath, Content).

+ 2 - 1
apps/emqx_ft/test/emqx_ft_SUITE.erl

@@ -270,7 +270,8 @@ t_nasty_filenames(_Config) ->
     Filenames = [
         {<<"nasty1">>, "146%"},
         {<<"nasty2">>, "🌚"},
-        {<<"nasty3">>, "中文.txt"}
+        {<<"nasty3">>, "中文.txt"},
+        {<<"nasty4">>, _254Bytes = string:join(lists:duplicate(255 div 5, "LONG"), ".")}
     ],
     ok = lists:foreach(
         fun({ClientId, Filename}) ->

+ 3 - 3
apps/emqx_ft/test/emqx_ft_conf_SUITE.erl

@@ -87,7 +87,7 @@ t_update_config(_Config) ->
         )
     ),
     ?assertEqual(
-        <<"/tmp/path">>,
+        "/tmp/path",
         emqx_config:get([file_transfer, storage, local, segments, root])
     ),
     ?assertEqual(
@@ -150,7 +150,7 @@ t_disable_restore_config(Config) ->
     ),
     ok = emqtt:stop(Client),
     % Restore local storage backend
-    Root = iolist_to_binary(emqx_ft_test_helpers:root(Config, node(), [segments])),
+    Root = emqx_ft_test_helpers:root(Config, node(), [segments]),
     ?assertMatch(
         {ok, _},
         emqx_conf:update(
@@ -177,7 +177,7 @@ t_disable_restore_config(Config) ->
                 [
                     #{
                         ?snk_kind := garbage_collection,
-                        storage := #{segments := #{root := Root}}
+                        storage := #{segments := #{gc := #{interval := 1000}}}
                     }
                 ],
                 ?of_kind(garbage_collection, Trace)

+ 21 - 0
apps/emqx_ft/test/emqx_ft_fs_util_tests.erl

@@ -63,3 +63,24 @@ unescape_filename_test_() ->
         ?_assertEqual(Input, emqx_ft_fs_util:unescape_filename(Filename))
      || {Filename, Input} <- ?NAMES
     ].
+
+mk_temp_filename_test_() ->
+    [
+        ?_assertMatch(
+            "." ++ Suffix when length(Suffix) == 16,
+            emqx_ft_fs_util:mk_temp_filename(<<>>)
+        ),
+        ?_assertMatch(
+            "file.name." ++ Suffix when length(Suffix) == 16,
+            emqx_ft_fs_util:mk_temp_filename("file.name")
+        ),
+        ?_assertMatch(
+            "safe.🦺." ++ Suffix when length(Suffix) == 16,
+            emqx_ft_fs_util:mk_temp_filename(<<"safe.🦺"/utf8>>)
+        ),
+        ?_assertEqual(
+            % FilenameSlice + Dot + Suffix
+            200 + 1 + 16,
+            length(emqx_ft_fs_util:mk_temp_filename(lists:duplicate(63, "LONG")))
+        )
+    ].