Merge pull request #10454 from keynslug/fix/EMQX-9157/limit-name-len

fix(ft): restrict max filename length in transfers

apps/emqx_ft/src/emqx_ft.erl

@@ -110,8 +110,8 @@ decode_filemeta(Map) when is_map(Map) ->
         Meta = hocon_tconf:check_plain(Schema, Map, #{atom_key => true, required => false}),
         {ok, Meta}
     catch
-        throw:Error ->
+        throw:{_Schema, Errors} ->
-            {error, {invalid_filemeta, Error}}
+            {error, {invalid_filemeta, Errors}}
     end.
 
 encode_filemeta(Meta = #{}) ->
@@ -381,7 +381,7 @@ do_validate([{filemeta, Payload} | Rest], Parsed) ->
         {ok, Meta} ->
             do_validate(Rest, [Meta | Parsed]);
         {error, Reason} ->
-            {error, {invalid_filemeta, Reason}}
+            {error, Reason}
     end;
 do_validate([{offset, Offset} | Rest], Parsed) ->
     case string:to_integer(Offset) of

apps/emqx_ft/src/emqx_ft_schema.erl

@@ -35,6 +35,13 @@
 
 -reflect_type([json_value/0]).
 
+%% NOTE
+%% This is rather conservative limit, mostly dictated by the filename limitations
+%% on most filesystems. Even though, say, S3 does not have such limitations, it's
+%% still useful to have a limit on the filename length, to avoid having to deal with
+%% limits in the storage backends.
+-define(MAX_FILENAME_BYTELEN, 255).
+
 -import(hoconsc, [ref/2, mk/2]).
 
 namespace() -> file_transfer.
@@ -234,7 +241,13 @@ schema(filemeta) ->
     }.
 
 validator(filename) ->
-    fun emqx_ft_fs_util:is_filename_safe/1.
+    [
+        fun(Value) ->
+            Bin = unicode:characters_to_binary(Value),
+            byte_size(Bin) =< ?MAX_FILENAME_BYTELEN orelse {error, max_length_exceeded}
+        end,
+        fun emqx_ft_fs_util:is_filename_safe/1
+    ].
 
 converter(checksum) ->
     fun

apps/emqx_ft/test/emqx_ft_SUITE.erl

@@ -183,9 +183,18 @@ t_invalid_filename(Config) ->
         unspecified_error,
         emqtt:publish(C, mk_init_topic(<<"f3">>), encode_meta(meta("/etc/passwd", <<>>)), 1)
     ),
+    ?assertRCName(
+        unspecified_error,
+        emqtt:publish(
+            C,
+            mk_init_topic(<<"f4">>),
+            encode_meta(meta(lists:duplicate(1000, $A), <<>>)),
+            1
+        )
+    ),
     ?assertRCName(
         success,
-        emqtt:publish(C, mk_init_topic(<<"f4">>), encode_meta(meta("146%", <<>>)), 1)
+        emqtt:publish(C, mk_init_topic(<<"f5">>), encode_meta(meta("146%", <<>>)), 1)
     ).
 
 t_simple_transfer(Config) ->

apps/emqx_s3/src/emqx_s3_schema.erl

@@ -12,6 +12,7 @@
 -export([roots/0, fields/1, namespace/0, tags/0, desc/1]).
 
 -export([translate/1]).
+-export([translate/2]).
 
 roots() ->
     [s3].
@@ -36,7 +37,8 @@ fields(s3) ->
                 string(),
                 #{
                     desc => ?DESC("secret_access_key"),
-                    required => false
+                    required => false,
+                    sensitive => true
                 }
             )},
         {bucket,
@@ -142,7 +144,10 @@ desc(transport_options) ->
     "Options for the HTTP transport layer used by the S3 client".
 
 translate(Conf) ->
-    Options = #{atom_key => true},
+    translate(Conf, #{}).
+
+translate(Conf, OptionsIn) ->
+    Options = maps:merge(#{atom_key => true}, OptionsIn),
     #{s3 := TranslatedConf} = hocon_tconf:check_plain(
         emqx_s3_schema, #{<<"s3">> => Conf}, Options, [s3]
     ),

apps/emqx_s3/test/emqx_s3_schema_SUITE.erl

@@ -108,6 +108,25 @@ t_full_config(_Config) ->
         })
     ).
 
+t_sensitive_config_hidden(_Config) ->
+    ?assertMatch(
+        #{
+            access_key_id := "access_key_id",
+            secret_access_key := <<"******">>
+        },
+        emqx_s3_schema:translate(
+            #{
+                <<"bucket">> => <<"bucket">>,
+                <<"host">> => <<"s3.us-east-1.endpoint.com">>,
+                <<"port">> => 443,
+                <<"access_key_id">> => <<"access_key_id">>,
+                <<"secret_access_key">> => <<"secret_access_key">>
+            },
+            % NOTE: this is what Config API handler is doing
+            #{obfuscate_sensitive_values => true}
+        )
+    ).
+
 t_invalid_limits(_Config) ->
     ?assertException(
         throw,