From 2f6b47e3ff10c1c4697cfdc5b86b083b2961ad81 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 15 Mar 2022 13:55:42 +0100 Subject: [PATCH] Report more friendly errors on invalid stanza id --- big_tests/tests/mam_SUITE.erl | 11 ++++++++++- src/mam/mod_mam.erl | 30 +++++++++++++++++++++++++----- src/mam/mod_mam_muc.erl | 24 ++++++++++++++++++++++-- src/mam/mod_mam_utils.erl | 4 +++- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/big_tests/tests/mam_SUITE.erl b/big_tests/tests/mam_SUITE.erl index e4755c9dab3..f568a60685a 100644 --- a/big_tests/tests/mam_SUITE.erl +++ b/big_tests/tests/mam_SUITE.erl @@ -81,6 +81,7 @@ pagination_offset5_opt_count_all/1, server_returns_item_not_found_for_before_filter_with_nonexistent_id/1, server_returns_item_not_found_for_after_filter_with_nonexistent_id/1, + server_returns_item_not_found_for_after_filter_with_invalid_id/1, %% complete_flag_cases tests before_complete_false_last5/1, before_complete_false_before10/1, @@ -500,7 +501,8 @@ rsm_cases() -> pagination_offset5_opt_count_all, %% item_not_found response for nonexistent message ID in before/after filters server_returns_item_not_found_for_before_filter_with_nonexistent_id, - server_returns_item_not_found_for_after_filter_with_nonexistent_id]. + server_returns_item_not_found_for_after_filter_with_nonexistent_id, + server_returns_item_not_found_for_after_filter_with_invalid_id]. complete_flag_cases() -> [before_complete_false_last5, @@ -2587,6 +2589,13 @@ server_returns_item_not_found_for_after_filter_with_nonexistent_id(Config) -> Condition = [<<"cancel">>, <<"item-not-found">>], server_returns_item_not_found_for_nonexistent_id(Config, RSM, StanzaID, Condition). +server_returns_item_not_found_for_after_filter_with_invalid_id(Config) -> + NonexistentID = <<"bef3a242-99ce-402a-9ffc-2f3c20da92d4">>, + RSM = #rsm_in{max = 5, direction = 'after', from_id = NonexistentID}, + StanzaID = <<"AV25E9SCO50K">>, + Condition = [<<"modify">>, <<"not-acceptable">>], + server_returns_item_not_found_for_nonexistent_id(Config, RSM, StanzaID, Condition). + server_returns_item_not_found_for_nonexistent_id(Config, RSM, StanzaID, Condition) -> P = ?config(props, Config), F = fun(Alice) -> diff --git a/src/mam/mod_mam.erl b/src/mam/mod_mam.erl index f3ccdd17d95..d187db4c2a9 100644 --- a/src/mam/mod_mam.erl +++ b/src/mam/mod_mam.erl @@ -106,7 +106,6 @@ -include("mongoose.hrl"). -include("jlib.hrl"). -include("amp.hrl"). --include_lib("exml/include/exml.hrl"). -include("mod_mam.hrl"). %% ---------------------------------------------------------------------- @@ -432,13 +431,29 @@ handle_get_prefs_result({error, Reason}, IQ) -> -spec handle_set_message_form(From :: jid:jid(), ArcJID :: jid:jid(), IQ :: jlib:iq(), Acc :: mongoose_acc:t()) -> jlib:iq() | ignore | {error, term(), jlib:iq()}. -handle_set_message_form(#jid{} = From, #jid{} = ArcJID, - #iq{xmlns=MamNs, sub_el = QueryEl} = IQ, - Acc) -> +handle_set_message_form(#jid{} = From, #jid{} = ArcJID, #iq{} = IQ, Acc) -> HostType = acc_to_host_type(Acc), ArcID = archive_id_int(HostType, ArcJID), + try iq_to_lookup_params(HostType, IQ) of + Params0 -> + do_handle_set_message_form(Params0, From, ArcID, ArcJID, IQ, HostType) + catch _C:R:S -> + report_issue({R, S}, mam_lookup_failed, ArcJID, IQ), + return_error_iq(IQ, R) + end. + + +-spec do_handle_set_message_form(Params :: mam_iq:lookup_params(), + From :: jid:jid(), + ArcId :: archive_id(), + ArcJID :: jid:jid(), + IQ :: jlib:iq(), + HostType :: mongooseim:host_type()) -> + jlib:iq() | ignore | {error, term(), jlib:iq()}. +do_handle_set_message_form(Params0, From, ArcID, ArcJID, + #iq{xmlns=MamNs, sub_el = QueryEl} = IQ, + HostType) -> QueryID = exml_query:attr(QueryEl, <<"queryid">>, <<>>), - Params0 = iq_to_lookup_params(HostType, IQ), Params = mam_iq:lookup_params_with_archive_details(Params0, ArcID, ArcJID, From), case lookup_messages(HostType, Params) of {error, Reason} -> @@ -683,6 +698,9 @@ return_error_iq(IQ, {Reason, {stacktrace, _Stacktrace}}) -> return_error_iq(IQ, timeout) -> E = mongoose_xmpp_errors:service_unavailable(<<"en">>, <<"Timeout">>), {error, timeout, IQ#iq{type = error, sub_el = [E]}}; +return_error_iq(IQ, invalid_stanza_id) -> + Text = mongoose_xmpp_errors:not_acceptable(<<"en">>, <<"Invalid stanza id provided">>), + {error, invalid_stanza_id, IQ#iq{type = error, sub_el = [Text]}}; return_error_iq(IQ, item_not_found) -> {error, item_not_found, IQ#iq{type = error, sub_el = [mongoose_xmpp_errors:item_not_found()]}}; return_error_iq(IQ, not_implemented) -> @@ -698,6 +716,8 @@ report_issue({Reason, {stacktrace, Stacktrace}}, Issue, ArcJID, IQ) -> report_issue(Reason, Issue, ArcJID, IQ) -> report_issue(Reason, [], Issue, ArcJID, IQ). +report_issue(invalid_stanza_id, _Stacktrace, _Issue, _ArcJID, _IQ) -> + expected; report_issue(item_not_found, _Stacktrace, _Issue, _ArcJID, _IQ) -> expected; report_issue(not_implemented, _Stacktrace, _Issue, _ArcJID, _IQ) -> diff --git a/src/mam/mod_mam_muc.erl b/src/mam/mod_mam_muc.erl index 284399fdc03..8e17fe45013 100644 --- a/src/mam/mod_mam_muc.erl +++ b/src/mam/mod_mam_muc.erl @@ -78,7 +78,6 @@ result_set/4, result_query/2, result_prefs/4, - make_fin_message/5, make_fin_element/7, parse_prefs/1, borders_decode/1, @@ -388,7 +387,23 @@ handle_set_message_form(HostType, #jid{} = From, #jid{} = ArcJID, IQ) -> ResLimit = mod_mam_params:max_result_limit(?MODULE, HostType), DefLimit = mod_mam_params:default_result_limit(?MODULE, HostType), ExtMod = mod_mam_params:extra_params_module(?MODULE, HostType), - Params0 = mam_iq:form_to_lookup_params(IQ, ResLimit, DefLimit, ExtMod), + try mam_iq:form_to_lookup_params(IQ, ResLimit, DefLimit, ExtMod) of + Params0 -> + do_handle_set_message_form(HostType, From, ArcID, ArcJID, IQ, Params0) + catch _C:R:S -> + report_issue({R, S}, mam_lookup_failed, ArcJID, IQ), + return_error_iq(IQ, R) + end. + + +-spec do_handle_set_message_form(HostType :: mongooseim:host_type(), + From :: jid:jid(), + ArcId :: mod_mam:archive_id(), + ArcJID :: jid:jid(), + IQ :: jlib:iq(), + Params :: mam_iq:lookup_params()) -> + jlib:iq() | ignore | {error, term(), jlib:iq()}. +do_handle_set_message_form(HostType, From, ArcID, ArcJID, IQ, Params0) -> Params = mam_iq:lookup_params_with_archive_details(Params0, ArcID, ArcJID, From), Result = lookup_messages(HostType, Params), handle_lookup_result(Result, HostType, From, IQ, Params). @@ -566,6 +581,9 @@ return_error_iq(IQ, {Reason, {stacktrace, _Stacktrace}}) -> return_error_iq(IQ, Reason); return_error_iq(IQ, timeout) -> {error, timeout, IQ#iq{type = error, sub_el = [mongoose_xmpp_errors:service_unavailable(<<"en">>, <<"Timeout in mod_mam_muc">>)]}}; +return_error_iq(IQ, invalid_stanza_id) -> + Text = mongoose_xmpp_errors:not_acceptable(<<"en">>, <<"Invalid stanza id provided">>), + {error, invalid_stanza_id, IQ#iq{type = error, sub_el = [Text]}}; return_error_iq(IQ, item_not_found) -> {error, item_not_found, IQ#iq{type = error, sub_el = [mongoose_xmpp_errors:item_not_found()]}}; return_error_iq(IQ, not_implemented) -> @@ -600,6 +618,8 @@ report_issue({Reason, {stacktrace, Stacktrace}}, Issue, ArcJID, IQ) -> report_issue(Reason, Issue, ArcJID, IQ) -> report_issue(Reason, [], Issue, ArcJID, IQ). +report_issue(invalid_stanza_id, _Stacktrace, _Issue, _ArcJID, _IQ) -> + expected; report_issue(item_not_found, _Stacktrace, _Issue, _ArcJID, _IQ) -> expected; report_issue(missing_with_jid, _Stacktrace, _Issue, _ArcJID, _IQ) -> diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl index a30121de5e7..ce2bcec6275 100644 --- a/src/mam/mod_mam_utils.erl +++ b/src/mam/mod_mam_utils.erl @@ -225,7 +225,9 @@ maybe_external_binary_to_mess_id(BExtMessID) -> %% @doc Decode a message ID received from the user. -spec external_binary_to_mess_id(binary()) -> integer(). external_binary_to_mess_id(BExtMessID) when is_binary(BExtMessID) -> - binary_to_integer(BExtMessID, 32). + try binary_to_integer(BExtMessID, 32) + catch error:badarg -> throw(invalid_stanza_id) + end. %% ----------------------------------------------------------------------- %% XML