From 67db9d6fe953a4cf45e3ed91fdfab6480d9700c4 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 5 Jun 2023 20:25:48 +0200 Subject: [PATCH 01/11] chore: bump version to 5.1.0-alpha.3 --- apps/emqx/include/emqx_release.hrl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx/include/emqx_release.hrl b/apps/emqx/include/emqx_release.hrl index 19854f40b..b128db61d 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -32,10 +32,10 @@ %% `apps/emqx/src/bpapi/README.md' %% Community edition --define(EMQX_RELEASE_CE, "5.1.0-alpha.2"). +-define(EMQX_RELEASE_CE, "5.1.0-alpha.3"). %% Enterprise edition --define(EMQX_RELEASE_EE, "5.1.0-alpha.2"). +-define(EMQX_RELEASE_EE, "5.1.0-alpha.3"). %% the HTTP API version -define(EMQX_API_VERSION, "5.0"). From f7a6648103d7527883548365d4c27c9d587c448d Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 5 Jun 2023 23:32:13 +0300 Subject: [PATCH 02/11] fix(mqttconn): no warn if ingress poolsize is same as config --- apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl b/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl index 86787b33b..a1cfe687f 100644 --- a/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl +++ b/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl @@ -99,7 +99,7 @@ choose_ingress_pool_size( {_Filter, #{share := _Name}} -> % NOTE: this is shared subscription, many workers may subscribe PoolSize; - {_Filter, #{}} -> + {_Filter, #{}} when PoolSize > 1 -> % NOTE: this is regular subscription, only one worker should subscribe ?SLOG(warning, #{ msg => "mqtt_bridge_ingress_pool_size_ignored", @@ -110,6 +110,8 @@ choose_ingress_pool_size( config_pool_size => PoolSize, pool_size => 1 }), + 1; + {_Filter, #{}} when PoolSize == 1 -> 1 end. From cd04b7cf8b24351ea231531143c52b273b26b897 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 5 Jun 2023 22:49:22 +0200 Subject: [PATCH 03/11] ci: skip github action cache for macos arm64 --- .github/actions/package-macos/action.yaml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/actions/package-macos/action.yaml b/.github/actions/package-macos/action.yaml index 9a19fa703..d27507dd0 100644 --- a/.github/actions/package-macos/action.yaml +++ b/.github/actions/package-macos/action.yaml @@ -43,8 +43,18 @@ runs: echo "OTP_SOURCE_PATH=$OTP_SOURCE_PATH" >> $GITHUB_OUTPUT echo "OTP_INSTALL_PATH=$OTP_INSTALL_PATH" >> $GITHUB_OUTPUT mkdir -p "$OTP_SOURCE_PATH" "$OTP_INSTALL_PATH" + # we need this to skip using cache for self-hosted runners + case ${{ inputs.os }} in + *arm64) + echo "SELF_HOSTED=true" >> $GITHUB_OUTPUT + ;; + *) + echo "SELF_HOSTED=false" >> $GITHUB_OUTPUT + ;; + esac - uses: actions/cache@v3 id: cache + if: steps.prepare.outputs.SELF_HOSTED != 'true' with: path: ${{ steps.prepare.outputs.OTP_INSTALL_PATH }} key: otp-install-${{ inputs.otp }}-${{ inputs.os }}-static-ssl-disable-hipe-disable-jit @@ -52,6 +62,17 @@ runs: if: steps.cache.outputs.cache-hit != 'true' shell: bash run: | + SELF_HOSTED="${{ steps.prepare.outputs.SELF_HOSTED}}" + # when it's self-hosted, it never hits the cache, + # skip rebuild if it's self-hosted and the install path already has a 'bin' + if [ "$SELF_HOSTED" = 'true' ]; then + if [ -d "$OTP_INSTALL_PATH/bin" ]; then + echo "Skip rebuilding OTP, found $OTP_INSTALL_PATH" + exit 0 + fi + fi + ## when it's not self-hosted, or the install path is not found, + ## build otp from source code. OTP_SOURCE_PATH="${{ steps.prepare.outputs.OTP_SOURCE_PATH }}" OTP_INSTALL_PATH="${{ steps.prepare.outputs.OTP_INSTALL_PATH }}" if [ -d "$OTP_SOURCE_PATH" ]; then From 541dc1b9cf6d68a2a020de94c311dfb835e8e12a Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 5 Jun 2023 23:19:34 +0200 Subject: [PATCH 04/11] ci: inspec erl in PATH for macos builds --- .github/actions/package-macos/action.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/actions/package-macos/action.yaml b/.github/actions/package-macos/action.yaml index d27507dd0..b53be1666 100644 --- a/.github/actions/package-macos/action.yaml +++ b/.github/actions/package-macos/action.yaml @@ -62,19 +62,19 @@ runs: if: steps.cache.outputs.cache-hit != 'true' shell: bash run: | - SELF_HOSTED="${{ steps.prepare.outputs.SELF_HOSTED}}" + OTP_SOURCE_PATH="${{ steps.prepare.outputs.OTP_SOURCE_PATH }}" + OTP_INSTALL_PATH="${{ steps.prepare.outputs.OTP_INSTALL_PATH }}" + SELF_HOSTED="${{ steps.prepare.outputs.SELF_HOSTED }}" # when it's self-hosted, it never hits the cache, # skip rebuild if it's self-hosted and the install path already has a 'bin' if [ "$SELF_HOSTED" = 'true' ]; then - if [ -d "$OTP_INSTALL_PATH/bin" ]; then + if [ -n "$OTP_INSTALL_PATH" && -d "$OTP_INSTALL_PATH/bin" ]; then echo "Skip rebuilding OTP, found $OTP_INSTALL_PATH" exit 0 fi fi ## when it's not self-hosted, or the install path is not found, ## build otp from source code. - OTP_SOURCE_PATH="${{ steps.prepare.outputs.OTP_SOURCE_PATH }}" - OTP_INSTALL_PATH="${{ steps.prepare.outputs.OTP_INSTALL_PATH }}" if [ -d "$OTP_SOURCE_PATH" ]; then rm -rf "$OTP_SOURCE_PATH" fi @@ -108,6 +108,10 @@ runs: shell: bash run: | export PATH="${{ steps.prepare.outputs.OTP_INSTALL_PATH }}/bin:$PATH" + # inspec erl in PATH + which erl + # inspec erl command banner + erl -s init stop make ensure-rebar3 mkdir -p $HOME/bin cp rebar3 $HOME/bin/rebar3 From b3c079dc21946d397b8e77213f4e9ea0620eb7a0 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 6 Jun 2023 01:21:09 +0200 Subject: [PATCH 05/11] ci: try --with-odbc --- .github/actions/package-macos/action.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/actions/package-macos/action.yaml b/.github/actions/package-macos/action.yaml index b53be1666..180621c67 100644 --- a/.github/actions/package-macos/action.yaml +++ b/.github/actions/package-macos/action.yaml @@ -67,8 +67,8 @@ runs: SELF_HOSTED="${{ steps.prepare.outputs.SELF_HOSTED }}" # when it's self-hosted, it never hits the cache, # skip rebuild if it's self-hosted and the install path already has a 'bin' - if [ "$SELF_HOSTED" = 'true' ]; then - if [ -n "$OTP_INSTALL_PATH" && -d "$OTP_INSTALL_PATH/bin" ]; then + if [ "${SELF_HOSTED:-false}" = 'true' ]; then + if [ -n "$OTP_INSTALL_PATH" ] && [ -d "$OTP_INSTALL_PATH/bin" ]; then echo "Skip rebuilding OTP, found $OTP_INSTALL_PATH" exit 0 fi @@ -81,10 +81,13 @@ runs: git clone --depth 1 --branch OTP-${{ inputs.otp }} https://github.com/emqx/otp.git "$OTP_SOURCE_PATH" cd "$OTP_SOURCE_PATH" if [ "$(arch)" = arm64 ]; then + export CFLAGS="-I$(brew --prefix unixodbc)/include" export LDFLAGS="-L$(brew --prefix unixodbc)/lib" - export CC="/usr/bin/gcc -I$(brew --prefix unixodbc)/include" + WITH_ODBC="--with-odbc=$(brew --prefix unixodbc)" + else + WITH_ODBC="" fi - ./configure --disable-dynamic-ssl-lib --with-ssl=$(brew --prefix openssl@1.1) --disable-hipe --disable-jit --prefix="$OTP_INSTALL_PATH" + ./configure --disable-dynamic-ssl-lib --with-ssl=$(brew --prefix openssl@1.1) ${WITH_ODBC} --disable-hipe --disable-jit --prefix="$OTP_INSTALL_PATH" make -j$(nproc) rm -rf "$OTP_INSTALL_PATH" make install From da8f3da4ccab7f5a65602692c9fc766f5d83d832 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 6 Jun 2023 01:45:51 +0200 Subject: [PATCH 06/11] ci: fix CFLAGS for macos otp build --- .github/actions/package-macos/action.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/package-macos/action.yaml b/.github/actions/package-macos/action.yaml index 180621c67..8615a433a 100644 --- a/.github/actions/package-macos/action.yaml +++ b/.github/actions/package-macos/action.yaml @@ -81,7 +81,7 @@ runs: git clone --depth 1 --branch OTP-${{ inputs.otp }} https://github.com/emqx/otp.git "$OTP_SOURCE_PATH" cd "$OTP_SOURCE_PATH" if [ "$(arch)" = arm64 ]; then - export CFLAGS="-I$(brew --prefix unixodbc)/include" + export CFLAGS="-O2 -g -I$(brew --prefix unixodbc)/include" export LDFLAGS="-L$(brew --prefix unixodbc)/lib" WITH_ODBC="--with-odbc=$(brew --prefix unixodbc)" else @@ -92,8 +92,8 @@ runs: rm -rf "$OTP_INSTALL_PATH" make install if [ "$(arch)" = arm64 ]; then + unset CFLAGS unset LDFLAGS - unset CC fi - name: build env: From 69b98c183078d7a0f49c6d61d42873dc86394ecd Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 5 Jun 2023 23:28:48 +0300 Subject: [PATCH 07/11] fix(ft): ensure temp filenames are under 255 bytes long --- apps/emqx_ft/src/emqx_ft_fs_util.erl | 23 +++++++++++++++++++ apps/emqx_ft/src/emqx_ft_schema.erl | 4 ++-- .../src/emqx_ft_storage_exporter_fs.erl | 3 +-- apps/emqx_ft/src/emqx_ft_storage_fs.erl | 12 ++-------- apps/emqx_ft/test/emqx_ft_SUITE.erl | 3 ++- apps/emqx_ft/test/emqx_ft_conf_SUITE.erl | 6 ++--- apps/emqx_ft/test/emqx_ft_fs_util_tests.erl | 21 +++++++++++++++++ 7 files changed, 54 insertions(+), 18 deletions(-) diff --git a/apps/emqx_ft/src/emqx_ft_fs_util.erl b/apps/emqx_ft/src/emqx_ft_fs_util.erl index 9028722aa..544585110 100644 --- a/apps/emqx_ft/src/emqx_ft_fs_util.erl +++ b/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(<>), + 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. diff --git a/apps/emqx_ft/src/emqx_ft_schema.erl b/apps/emqx_ft/src/emqx_ft_schema.erl index 37508fe3e..2b98562b4 100644 --- a/apps/emqx_ft/src/emqx_ft_schema.erl +++ b/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 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 702bc35ce..ae709dd52 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl +++ b/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl @@ -452,8 +452,7 @@ mk_manifest_filename(Filename) when is_binary(Filename) -> <>. 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) -> diff --git a/apps/emqx_ft/src/emqx_ft_storage_fs.erl b/apps/emqx_ft/src/emqx_ft_storage_fs.erl index 99720f521..e84d35328 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_fs.erl +++ b/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). diff --git a/apps/emqx_ft/test/emqx_ft_SUITE.erl b/apps/emqx_ft/test/emqx_ft_SUITE.erl index ae274cd86..6861038e9 100644 --- a/apps/emqx_ft/test/emqx_ft_SUITE.erl +++ b/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}) -> diff --git a/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl b/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl index 1f53f88af..f235b5ebb 100644 --- a/apps/emqx_ft/test/emqx_ft_conf_SUITE.erl +++ b/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) diff --git a/apps/emqx_ft/test/emqx_ft_fs_util_tests.erl b/apps/emqx_ft/test/emqx_ft_fs_util_tests.erl index 1939e74c6..f26dea7e7 100644 --- a/apps/emqx_ft/test/emqx_ft_fs_util_tests.erl +++ b/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"))) + ) + ]. From dcd59e4f1b069498dd669d9f4efbffa51a624f79 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Tue, 6 Jun 2023 10:40:20 +0300 Subject: [PATCH 08/11] fix(ft): set more conservative filename length limit Otherwise, local fs exporter will have a hard time preserving the filemeta, because its filename is even 13 bytes longer. --- apps/emqx_ft/src/emqx_ft_schema.erl | 4 +++- apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl | 12 +++++++++++- apps/emqx_ft/test/emqx_ft_SUITE.erl | 4 +++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/apps/emqx_ft/src/emqx_ft_schema.erl b/apps/emqx_ft/src/emqx_ft_schema.erl index 2b98562b4..c1ee41d0d 100644 --- a/apps/emqx_ft/src/emqx_ft_schema.erl +++ b/apps/emqx_ft/src/emqx_ft_schema.erl @@ -42,7 +42,9 @@ %% 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). +%% Usual realistic limit is 255 bytes actually, but we leave some room for backends +%% to spare. +-define(MAX_FILENAME_BYTELEN, 240). -import(hoconsc, [ref/2, mk/2]). 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 ae709dd52..e211cb421 100644 --- a/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl +++ b/apps/emqx_ft/src/emqx_ft_storage_exporter_fs.erl @@ -128,7 +128,17 @@ complete( Filemeta = FilemetaIn#{checksum => Checksum}, ok = file:close(Handle), _ = filelib:ensure_dir(ResultFilepath), - _ = file:write_file(mk_manifest_filename(ResultFilepath), encode_filemeta(Filemeta)), + ManifestFilepath = mk_manifest_filename(ResultFilepath), + case file:write_file(ManifestFilepath, encode_filemeta(Filemeta)) of + ok -> + ok; + {error, Reason} -> + ?SLOG(warning, "filemeta_write_failed", #{ + path => ManifestFilepath, + meta => Filemeta, + reason => Reason + }) + end, file:rename(Filepath, ResultFilepath). -spec discard(export_st()) -> diff --git a/apps/emqx_ft/test/emqx_ft_SUITE.erl b/apps/emqx_ft/test/emqx_ft_SUITE.erl index 6861038e9..c48c77d93 100644 --- a/apps/emqx_ft/test/emqx_ft_SUITE.erl +++ b/apps/emqx_ft/test/emqx_ft_SUITE.erl @@ -261,6 +261,7 @@ t_nasty_clientids_fileids(_Config) -> fun({ClientId, FileId}) -> ok = emqx_ft_test_helpers:upload_file(ClientId, FileId, "justfile", ClientId), [Export] = list_files(ClientId), + ?assertMatch(#{meta := #{name := "justfile"}}, Export), ?assertEqual({ok, ClientId}, read_export(Export)) end, Transfers @@ -271,13 +272,14 @@ t_nasty_filenames(_Config) -> {<<"nasty1">>, "146%"}, {<<"nasty2">>, "🌚"}, {<<"nasty3">>, "中文.txt"}, - {<<"nasty4">>, _254Bytes = string:join(lists:duplicate(255 div 5, "LONG"), ".")} + {<<"nasty4">>, _239Bytes = string:join(lists:duplicate(240 div 5, "LONG"), ".")} ], 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), + ?assertMatch(#{meta := #{name := Filename}}, Export), ?assertEqual({ok, FileId}, read_export(Export)) end, Filenames From bcc47442eb10d980a11a8ddf254f6523a4d781f7 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Tue, 6 Jun 2023 18:44:27 +0800 Subject: [PATCH 09/11] fix(mqttsn): make mountpoint works for publish --- .../src/emqx_gateway_mqttsn.app.src | 2 +- .../src/emqx_mqttsn_channel.erl | 5 ++- .../test/emqx_sn_protocol_SUITE.erl | 45 +++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/apps/emqx_gateway_mqttsn/src/emqx_gateway_mqttsn.app.src b/apps/emqx_gateway_mqttsn/src/emqx_gateway_mqttsn.app.src index 76f0f45b5..b43201e1a 100644 --- a/apps/emqx_gateway_mqttsn/src/emqx_gateway_mqttsn.app.src +++ b/apps/emqx_gateway_mqttsn/src/emqx_gateway_mqttsn.app.src @@ -1,6 +1,6 @@ {application, emqx_gateway_mqttsn, [ {description, "MQTT-SN Gateway"}, - {vsn, "0.1.1"}, + {vsn, "0.1.2"}, {registered, []}, {applications, [kernel, stdlib, emqx, emqx_gateway]}, {env, []}, diff --git a/apps/emqx_gateway_mqttsn/src/emqx_mqttsn_channel.erl b/apps/emqx_gateway_mqttsn/src/emqx_mqttsn_channel.erl index 914f837e1..84334875b 100644 --- a/apps/emqx_gateway_mqttsn/src/emqx_mqttsn_channel.erl +++ b/apps/emqx_gateway_mqttsn/src/emqx_mqttsn_channel.erl @@ -1111,15 +1111,16 @@ check_pub_authz( convert_pub_to_msg( {TopicName, Flags, Data}, - Channel = #channel{clientinfo = #{clientid := ClientId}} + Channel = #channel{clientinfo = #{clientid := ClientId, mountpoint := Mountpoint}} ) -> #mqtt_sn_flags{qos = QoS, dup = Dup, retain = Retain} = Flags, NewQoS = get_corrected_qos(QoS), + NTopicName = emqx_mountpoint:mount(Mountpoint, TopicName), Message = put_message_headers( emqx_message:make( ClientId, NewQoS, - TopicName, + NTopicName, Data, #{dup => Dup, retain => Retain}, #{} diff --git a/apps/emqx_gateway_mqttsn/test/emqx_sn_protocol_SUITE.erl b/apps/emqx_gateway_mqttsn/test/emqx_sn_protocol_SUITE.erl index cce4ce904..b22d7b4b0 100644 --- a/apps/emqx_gateway_mqttsn/test/emqx_sn_protocol_SUITE.erl +++ b/apps/emqx_gateway_mqttsn/test/emqx_sn_protocol_SUITE.erl @@ -120,6 +120,13 @@ restart_mqttsn_with_subs_resume_off() -> Conf#{<<"subs_resume">> => <<"false">>} ). +restart_mqttsn_with_mountpoint(Mp) -> + Conf = emqx:get_raw_config([gateway, mqttsn]), + emqx_gateway_conf:update_gateway( + mqttsn, + Conf#{<<"mountpoint">> => Mp} + ). + default_config() -> ?CONF_DEFAULT. @@ -990,6 +997,44 @@ t_publish_qos2_case03(_) -> ?assertEqual(<<2, ?SN_DISCONNECT>>, receive_response(Socket)), gen_udp:close(Socket). +t_publish_mountpoint(_) -> + restart_mqttsn_with_mountpoint(<<"mp/">>), + Dup = 0, + QoS = 1, + Retain = 0, + Will = 0, + CleanSession = 0, + MsgId = 1, + TopicId1 = ?MAX_PRED_TOPIC_ID + 1, + Topic = <<"abc">>, + {ok, Socket} = gen_udp:open(0, [binary]), + ClientId = ?CLIENTID, + send_connect_msg(Socket, ClientId), + ?assertEqual(<<3, ?SN_CONNACK, 0>>, receive_response(Socket)), + send_subscribe_msg_normal_topic(Socket, QoS, Topic, MsgId), + ?assertEqual( + <<8, ?SN_SUBACK, Dup:1, QoS:2, Retain:1, Will:1, CleanSession:1, ?SN_NORMAL_TOPIC:2, + TopicId1:16, MsgId:16, ?SN_RC_ACCEPTED>>, + receive_response(Socket) + ), + + Payload1 = <<20, 21, 22, 23>>, + send_publish_msg_normal_topic(Socket, QoS, MsgId, TopicId1, Payload1), + ?assertEqual( + <<7, ?SN_PUBACK, TopicId1:16, MsgId:16, ?SN_RC_ACCEPTED>>, receive_response(Socket) + ), + timer:sleep(100), + + ?assertEqual( + <<11, ?SN_PUBLISH, Dup:1, QoS:2, Retain:1, Will:1, CleanSession:1, ?SN_NORMAL_TOPIC:2, + TopicId1:16, MsgId:16, <<20, 21, 22, 23>>/binary>>, + receive_response(Socket) + ), + + send_disconnect_msg(Socket, undefined), + restart_mqttsn_with_mountpoint(<<>>), + gen_udp:close(Socket). + t_delivery_qos1_register_invalid_topic_id(_) -> Dup = 0, QoS = 1, From f26b372e51507aca327b9d04af2a7b5c3a5292b4 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Tue, 6 Jun 2023 18:47:41 +0800 Subject: [PATCH 10/11] chore: update changes --- changes/ce/fix-10951.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/fix-10951.en.md diff --git a/changes/ce/fix-10951.en.md b/changes/ce/fix-10951.en.md new file mode 100644 index 000000000..89dabb4a7 --- /dev/null +++ b/changes/ce/fix-10951.en.md @@ -0,0 +1 @@ +Fix the issue in MQTT-SN gateway where the `mountpoint` does not take effect on message publishing. From 1968589f81d543b8c6b68c855b0c5e488c9f7e4d Mon Sep 17 00:00:00 2001 From: Serge Tupchii Date: Tue, 6 Jun 2023 14:02:58 +0300 Subject: [PATCH 11/11] fix(emqx_schema): don't allow enabling `fail_if_no_peer_cert` if `verify_none` is set Setting `fail_if_no_peer_cert = true` and `verify = verify_none` causes connection errors. Closes: EMQX-9586 --- apps/emqx/src/emqx_schema.erl | 18 +++++- apps/emqx/test/emqx_schema_tests.erl | 61 +++++++++++++++++++ apps/emqx_gateway/src/emqx_gateway_schema.erl | 13 +++- changes/ce/fix-10952.en.md | 8 +++ 4 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 changes/ce/fix-10952.en.md diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 37d5350a5..521293f7a 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -94,7 +94,8 @@ validate_keepalive_multiplier/1, non_empty_string/1, validations/0, - naive_env_interpolation/1 + naive_env_interpolation/1, + validate_server_ssl_opts/1 ]). -export([qos/0]). @@ -958,7 +959,7 @@ fields("mqtt_wss_listener") -> {"ssl_options", sc( ref("listener_wss_opts"), - #{} + #{validator => fun validate_server_ssl_opts/1} )}, {"websocket", sc( @@ -2426,8 +2427,21 @@ server_ssl_opts_schema(Defaults, IsRanchListener) -> ] ]. +validate_server_ssl_opts(#{<<"fail_if_no_peer_cert">> := true, <<"verify">> := Verify}) -> + validate_verify(Verify); +validate_server_ssl_opts(#{fail_if_no_peer_cert := true, verify := Verify}) -> + validate_verify(Verify); +validate_server_ssl_opts(_SSLOpts) -> + ok. + +validate_verify(verify_peer) -> + ok; +validate_verify(_) -> + {error, "verify must be verify_peer when fail_if_no_peer_cert is true"}. + mqtt_ssl_listener_ssl_options_validator(Conf) -> Checks = [ + fun validate_server_ssl_opts/1, fun ocsp_outer_validator/1, fun crl_outer_validator/1 ], diff --git a/apps/emqx/test/emqx_schema_tests.erl b/apps/emqx/test/emqx_schema_tests.erl index ad2341460..58f9a94d5 100644 --- a/apps/emqx/test/emqx_schema_tests.erl +++ b/apps/emqx/test/emqx_schema_tests.erl @@ -106,6 +106,67 @@ bad_cipher_test() -> ), ok. +fail_if_no_peer_cert_test_() -> + Sc = #{ + roots => [mqtt_ssl_listener], + fields => #{mqtt_ssl_listener => emqx_schema:fields("mqtt_ssl_listener")} + }, + Opts = #{atom_key => false, required => false}, + OptsAtomKey = #{atom_key => true, required => false}, + InvalidConf = #{ + <<"bind">> => <<"0.0.0.0:9883">>, + <<"ssl_options">> => #{ + <<"fail_if_no_peer_cert">> => true, + <<"verify">> => <<"verify_none">> + } + }, + InvalidListener = #{<<"mqtt_ssl_listener">> => InvalidConf}, + ValidListener = #{ + <<"mqtt_ssl_listener">> => InvalidConf#{ + <<"ssl_options">> => + #{ + <<"fail_if_no_peer_cert">> => true, + <<"verify">> => <<"verify_peer">> + } + } + }, + ValidListener1 = #{ + <<"mqtt_ssl_listener">> => InvalidConf#{ + <<"ssl_options">> => + #{ + <<"fail_if_no_peer_cert">> => false, + <<"verify">> => <<"verify_none">> + } + } + }, + Reason = "verify must be verify_peer when fail_if_no_peer_cert is true", + [ + ?_assertThrow( + {_Sc, [#{kind := validation_error, reason := Reason}]}, + hocon_tconf:check_plain(Sc, InvalidListener, Opts) + ), + ?_assertThrow( + {_Sc, [#{kind := validation_error, reason := Reason}]}, + hocon_tconf:check_plain(Sc, InvalidListener, OptsAtomKey) + ), + ?_assertMatch( + #{mqtt_ssl_listener := #{}}, + hocon_tconf:check_plain(Sc, ValidListener, OptsAtomKey) + ), + ?_assertMatch( + #{mqtt_ssl_listener := #{}}, + hocon_tconf:check_plain(Sc, ValidListener1, OptsAtomKey) + ), + ?_assertMatch( + #{<<"mqtt_ssl_listener">> := #{}}, + hocon_tconf:check_plain(Sc, ValidListener, Opts) + ), + ?_assertMatch( + #{<<"mqtt_ssl_listener">> := #{}}, + hocon_tconf:check_plain(Sc, ValidListener1, Opts) + ) + ]. + validate(Schema, Data0) -> Sc = #{ roots => [ssl_opts], diff --git a/apps/emqx_gateway/src/emqx_gateway_schema.erl b/apps/emqx_gateway/src/emqx_gateway_schema.erl index 8c80fc1fa..3c5706e82 100644 --- a/apps/emqx_gateway/src/emqx_gateway_schema.erl +++ b/apps/emqx_gateway/src/emqx_gateway_schema.erl @@ -120,7 +120,10 @@ fields(ssl_listener) -> {ssl_options, sc( hoconsc:ref(emqx_schema, "listener_ssl_opts"), - #{desc => ?DESC(ssl_listener_options)} + #{ + desc => ?DESC(ssl_listener_options), + validator => fun emqx_schema:validate_server_ssl_opts/1 + } )} ]; fields(udp_listener) -> @@ -132,7 +135,13 @@ fields(udp_listener) -> fields(dtls_listener) -> [{acceptors, sc(integer(), #{default => 16, desc => ?DESC(dtls_listener_acceptors)})}] ++ fields(udp_listener) ++ - [{dtls_options, sc(ref(dtls_opts), #{desc => ?DESC(dtls_listener_dtls_opts)})}]; + [ + {dtls_options, + sc(ref(dtls_opts), #{ + desc => ?DESC(dtls_listener_dtls_opts), + validator => fun emqx_schema:validate_server_ssl_opts/1 + })} + ]; fields(udp_opts) -> [ {active_n, diff --git a/changes/ce/fix-10952.en.md b/changes/ce/fix-10952.en.md new file mode 100644 index 000000000..20792906f --- /dev/null +++ b/changes/ce/fix-10952.en.md @@ -0,0 +1,8 @@ +Disallow enabling `fail_if_no_peer_cert` in listener SSL options if `verify_none` is set. + +Setting `fail_if_no_peer_cert = true` and `verify = verify_none` caused connection errors +due to incompatible options. +This fix validates the options when creating or updating a listener to avoid these errors. + +Note: any old listener configuration with `fail_if_no_peer_cert = true` and `verify = verify_none` +that was previously allowed will fail to load after applying this fix and must be manually fixed.