From cb14a3e08b051fe291fd71e8b12da84aaaba344a Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Tue, 23 May 2023 19:58:05 +0300 Subject: [PATCH 1/3] fix(ft): handle empty filepath in fs exporter API Fixes EMQX-9973 --- .../src/emqx_ft_storage_exporter_fs_api.erl | 2 +- apps/emqx_ft/test/emqx_ft_api_SUITE.erl | 28 +++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs_api.erl b/apps/emqx_ft/src/emqx_ft_storage_exporter_fs_api.erl index abb774f82..40944c0e8 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs_api.erl +++ b/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 -> diff --git a/apps/emqx_ft/test/emqx_ft_api_SUITE.erl b/apps/emqx_ft/test/emqx_ft_api_SUITE.erl index f69e13a6d..2988e0083 100644 --- a/apps/emqx_ft/test/emqx_ft_api_SUITE.erl +++ b/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()}) ) ), From 2dbf84479c14a96991a6dd98500e6761cef14c20 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Tue, 23 May 2023 20:00:00 +0300 Subject: [PATCH 2/3] fix(ft): handle wider class of jiffy decode errors With malformed cursors in the File listing API. Fixes EMQX-9965 --- apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl | 2 +- apps/emqx_ft/test/emqx_ft_api_SUITE.erl | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl b/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl index 6738d6fef..702bc35ce 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl +++ b/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}); diff --git a/apps/emqx_ft/test/emqx_ft_api_SUITE.erl b/apps/emqx_ft/test/emqx_ft_api_SUITE.erl index 2988e0083..18a8e9841 100644 --- a/apps/emqx_ft/test/emqx_ft_api_SUITE.erl +++ b/apps/emqx_ft/test/emqx_ft_api_SUITE.erl @@ -216,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( From f4047d3946aac38d0e38986c2db025e1fc039090 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Tue, 23 May 2023 20:46:47 +0300 Subject: [PATCH 3/3] test(ft): add testcase for nasty tranfer filenames * Emoji * Chinese characters --- apps/emqx_ft/test/emqx_ft_SUITE.erl | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/apps/emqx_ft/test/emqx_ft_SUITE.erl b/apps/emqx_ft/test/emqx_ft_SUITE.erl index 7d64f9716..e582db01f 100644 --- a/apps/emqx_ft/test/emqx_ft_SUITE.erl +++ b/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),