Skip to content

Commit

Permalink
Fix routing messages to bare jids (#1512)
Browse files Browse the repository at this point in the history
* Fix routing messages to bare jids

The `Packet` has to be passed to mongoose_acc:strip in this case.
Otherwise the recipient will get the original stanza from Acc which
may not be correct.
Simple case:
1. `User A` sends and IQ to MongooseIM
2. This IQ results in a `message` stanza to `User B` (bare jid)
3. Withtout this change `User B` recieves the IQ sent by `User A`

* Add a test case for mongoose_acc:strip/2 usage in ejabberd_sm

* Fix acc_e2e_SUITE teardown
  • Loading branch information
michalwski authored and fenek committed Oct 23, 2017
1 parent cf287ff commit 340a926
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 5 deletions.
2 changes: 1 addition & 1 deletion apps/ejabberd/src/ejabberd_sm.erl
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ route_message(From, To, Acc, Packet) ->
%% positive
fun({Prio, Pid}) when Prio == Priority ->
%% we will lose message if PID is not alive
Pid ! {route, From, To, mongoose_acc:strip(Acc)};
Pid ! {route, From, To, mongoose_acc:strip(Acc, Packet)};
%% Ignore other priority:
({_Prio, _Pid}) ->
ok
Expand Down
1 change: 1 addition & 0 deletions test.disabled/ejabberd_tests/default.spec
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
%% do not remove below SUITE if testing mongoose
{suites, "tests", mongoose_sanity_checks_SUITE}.

{suites, "tests", acc_e2e_SUITE}.
{suites, "tests", adhoc_SUITE}.
{suites, "tests", amp_big_SUITE}.
{suites, "tests", anonymous_SUITE}.
Expand Down
27 changes: 24 additions & 3 deletions test.disabled/ejabberd_tests/tests/acc_e2e_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ all() ->

groups() ->
[
{message, [parallel], message_test_cases()}
{message, [], message_test_cases()}
].

message_test_cases() ->
[
one_message
one_message,
message_altered_by_filter_local_packet_hook
].

suite() ->
Expand Down Expand Up @@ -74,15 +75,22 @@ init_per_group(_GroupName, Config) ->
escalus:create_users(Config, escalus:get_users([alice, bob])).

end_per_group(message, Config) ->
remove_handler(c2s_preprocessing_hook, test_preprocess, 50),
remove_handler(c2s_preprocessing_hook, test_save_acc, 50),
remove_handler(filter_local_packet, test_check_acc, 50),
remove_handler(user_receive_packet, test_check_final_acc, 50),
escalus:delete_users(Config, escalus:get_users([alice, bob]));
end_per_group(_GroupName, Config) ->
escalus:delete_users(Config, escalus:get_users([alice, bob])).

init_per_testcase(message_altered_by_filter_local_packet_hook = CN, Config) ->
add_handler(filter_local_packet, alter_message, 60),
escalus:init_per_testcase(CN, Config);
init_per_testcase(CaseName, Config) ->
escalus:init_per_testcase(CaseName, Config).

end_per_testcase(message_altered_by_filter_local_packet_hook = CN, Config) ->
remove_handler(filter_local_packet, alter_message, 60),
escalus:end_per_testcase(CN, Config);
end_per_testcase(CaseName, Config) ->
escalus:end_per_testcase(CaseName, Config).

Expand All @@ -102,6 +110,19 @@ one_message(Config) ->
ok
end).

message_altered_by_filter_local_packet_hook(Config) ->
escalus:fresh_story(
Config, [{alice, 1}, {bob, 1}],
fun(Alice, Bob) ->
M = escalus_stanza:chat_to(escalus_client:short_jid(Bob), <<"hi">>),
escalus:send(Alice, M),
R = escalus_client:wait_for_stanza(Bob),
% filter_local_packet alters packet to be delivered
% and mongoose_acc:strip must respect it
escalus:assert(is_chat_message, [<<"bye">>], R),
ok
end).

acc_test_helper_code(Config) ->
Dir = proplists:get_value(data_dir, Config),
F = filename:join(Dir, "acc_test_helper.erl"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
-module(acc_test_helper).
-compile(export_all).
-author("bartek").

-compile(export_all).

test_save_acc(#{type := <<"chat">>} = Acc, _State) ->
Rand = rand:uniform(),
Expand Down Expand Up @@ -46,3 +46,10 @@ check_acc(Acc, stripped) ->
undefined = mongoose_acc:get(should_be_stripped, Acc, undefined),
check_acc(Acc).

alter_message({From, To, Acc, Packet}) ->
% Not using #xmlel as it causes some strange error in dynamic compilation
{xmlel, PName, PAttrs, PCh} = Packet,
NewBody = {xmlel, <<"body">>, [], [{xmlcdata, <<"bye">> }]},
PCh2 = lists:keyreplace(<<"body">>, 2, PCh, NewBody),
{From, To, Acc, {xmlel, PName, PAttrs, PCh2}}.

0 comments on commit 340a926

Please sign in to comment.