Bladeren bron

Merge pull request #10793 from fix/EMQX-9965/bugs

fix(ft): handle wider class of bad input in APIs
Andrew Mayorov 2 jaren geleden
bovenliggende
commit
3fd2887921

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

@@ -422,7 +422,7 @@ decode_cursor(Cursor) ->
         true = is_list(Name),
         {Node, #{transfer => {ClientId, FileId}, name => Name}}
     catch
-        error:{_, invalid_json} ->
+        error:{Loc, JsonError} when is_integer(Loc), is_atom(JsonError) ->
             error({badarg, cursor});
         error:{badmatch, _} ->
             error({badarg, cursor});

+ 1 - 1
apps/emqx_ft/src/emqx_ft_storage_exporter_fs_api.erl

@@ -167,7 +167,7 @@ parse_filepath(PathBin) ->
             throw({invalid, PathBin})
     end,
     PathComponents = filename:split(PathBin),
-    case lists:any(fun is_special_component/1, PathComponents) of
+    case PathComponents == [] orelse lists:any(fun is_special_component/1, PathComponents) of
         false ->
             filename:join(PathComponents);
         true ->

+ 17 - 4
apps/emqx_ft/test/emqx_ft_SUITE.erl

@@ -47,6 +47,7 @@ groups() ->
             t_invalid_topic_format,
             t_meta_conflict,
             t_nasty_clientids_fileids,
+            t_nasty_filenames,
             t_no_meta,
             t_no_segment,
             t_simple_transfer
@@ -205,10 +206,6 @@ t_invalid_filename(Config) ->
             encode_meta(meta(lists:duplicate(1000, $A), <<>>)),
             1
         )
-    ),
-    ?assertRCName(
-        success,
-        emqtt:publish(C, mk_init_topic(<<"f5">>), encode_meta(meta("146%", <<>>)), 1)
     ).
 
 t_simple_transfer(Config) ->
@@ -265,6 +262,22 @@ t_nasty_clientids_fileids(_Config) ->
         Transfers
     ).
 
+t_nasty_filenames(_Config) ->
+    Filenames = [
+        {<<"nasty1">>, "146%"},
+        {<<"nasty2">>, "🌚"},
+        {<<"nasty3">>, "中文.txt"}
+    ],
+    ok = lists:foreach(
+        fun({ClientId, Filename}) ->
+            FileId = unicode:characters_to_binary(Filename),
+            ok = emqx_ft_test_helpers:upload_file(ClientId, FileId, Filename, FileId),
+            [Export] = list_files(ClientId),
+            ?assertEqual({ok, FileId}, read_export(Export))
+        end,
+        Filenames
+    ).
+
 t_meta_conflict(Config) ->
     C = ?config(client, Config),
 

+ 30 - 8
apps/emqx_ft/test/emqx_ft_api_SUITE.erl

@@ -140,10 +140,7 @@ t_download_transfer(Config) ->
         request(
             get,
             uri(["file_transfer", "file"]) ++
-                query(#{
-                    fileref => FileId,
-                    node => <<"nonode@nohost">>
-                })
+                query(#{fileref => FileId, node => <<"nonode@nohost">>})
         )
     ),
 
@@ -152,10 +149,25 @@ t_download_transfer(Config) ->
         request(
             get,
             uri(["file_transfer", "file"]) ++
-                query(#{
-                    fileref => <<"unknown_file">>,
-                    node => node()
-                })
+                query(#{fileref => <<"unknown_file">>, node => node()})
+        )
+    ),
+
+    ?assertMatch(
+        {ok, 404, #{<<"message">> := <<"Invalid query parameter", _/bytes>>}},
+        request_json(
+            get,
+            uri(["file_transfer", "file"]) ++
+                query(#{fileref => <<>>, node => node()})
+        )
+    ),
+
+    ?assertMatch(
+        {ok, 404, #{<<"message">> := <<"Invalid query parameter", _/bytes>>}},
+        request_json(
+            get,
+            uri(["file_transfer", "file"]) ++
+                query(#{fileref => <<"/etc/passwd">>, node => node()})
         )
     ),
 
@@ -204,6 +216,16 @@ t_list_files_paging(Config) ->
         request_json(get, uri(["file_transfer", "files"]) ++ query(#{limit => 0}))
     ),
 
+    ?assertMatch(
+        {ok, 400, #{<<"code">> := <<"BAD_REQUEST">>}},
+        request_json(get, uri(["file_transfer", "files"]) ++ query(#{following => <<>>}))
+    ),
+
+    ?assertMatch(
+        {ok, 400, #{<<"code">> := <<"BAD_REQUEST">>}},
+        request_json(get, uri(["file_transfer", "files"]) ++ query(#{following => <<"{\"\":}">>}))
+    ),
+
     ?assertMatch(
         {ok, 400, #{<<"code">> := <<"BAD_REQUEST">>}},
         request_json(