-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
c2s/mod_last #3750
c2s/mod_last #3750
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
7b68709
to
2a5694a
Compare
10e4f53
to
9b9f2da
Compare
This comment was marked as outdated.
This comment was marked as outdated.
c6105e8
to
7b8e99c
Compare
1edaa0f
to
4152aa9
Compare
9b9f2da
to
b5a0c35
Compare
This comment was marked as outdated.
This comment was marked as outdated.
b5a0c35
to
49e6f5b
Compare
ldap_mnesia_24 / ldap_mnesia / 49e6f5b small_tests_24 / small_tests / 49e6f5b ldap_mnesia_25 / ldap_mnesia / 49e6f5b small_tests_25 / small_tests / 49e6f5b dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 49e6f5b internal_mnesia_25 / internal_mnesia / 49e6f5b pgsql_mnesia_24 / pgsql_mnesia / 49e6f5b elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 49e6f5b dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 49e6f5b mysql_redis_25 / mysql_redis / 49e6f5b metrics_session_SUITE:session_global:session_global{error,
{{totalSessionCount,
{value,1},
[{times,25,
{error,
{badmatch,{value,0}},
[{metrics_helper,assert_counter,3,
[{file,
"/home/circleci/project/big_tests/tests/metrics_helper.erl"},
{line,36}]},
{mongoose_helper,do_wait_until,2,
[{file,
"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,374}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,
[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}]},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,371}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} pgsql_mnesia_25 / pgsql_mnesia / 49e6f5b dynamic_domains_mysql_redis_25 / mysql_redis / 49e6f5b dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 49e6f5b riak_mnesia_24 / riak_mnesia / 49e6f5b mssql_mnesia_25 / odbc_mssql_mnesia / 49e6f5b |
Codecov ReportBase: 57.75% // Head: 57.80% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## feature/mongoose_c2s #3750 +/- ##
========================================================
+ Coverage 57.75% 57.80% +0.05%
========================================================
Files 536 536
Lines 34809 34822 +13
========================================================
+ Hits 20103 20129 +26
+ Misses 14706 14693 -13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a few comments, please provide PR descriptions.
src/mod_last.erl
Outdated
@@ -46,6 +46,7 @@ | |||
session_cleanup/5, | |||
remove_domain/3, | |||
remove_unused_backend_opts/1]). | |||
-export([user_receive_iq/3]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: why is this one exported separately? The above are already IQ and hook handlers (two types of handlers), and they are exported together. So it looks like a minor inconsistency.
src/mod_last.erl
Outdated
{From, To, _} = mongoose_acc:packet(Acc), | ||
case {To#jid.lresource, can_respond(HostType, From, To)} of | ||
{<<>>, _} -> | ||
{ok, Acc}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this clause reachable? Wouldn't it be caught by the IQ handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that is true!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XMPP core itself says that IQs to a user without specifying the resource must be handled by the server on their behalf: https://www.rfc-editor.org/rfc/rfc6120#section-10.5.3.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
This module now implements a handler for an online user receiving an IQ, which used to be in `ejabberd_c2s:handle_routed_iq/4`. This happens when the IQ is directed to a full JID with a resource, but it still needs to check if it is allowed to be routed, according to XEP-0012 saying: > if the requesting entity is not authorized to view the user's presence > information (normally via a presence subscription as defined in XMPP > IM), the user's server MUST NOT deliver the IQ-get to an available > resource but instead MUST return a <forbidden/> error in response to > the last activity request. https://xmpp.org/extensions/xep-0012.html#online
49e6f5b
to
7985965
Compare
small_tests_24 / small_tests / 7985965 ldap_mnesia_24 / ldap_mnesia / 7985965 ldap_mnesia_25 / ldap_mnesia / 7985965 small_tests_25 / small_tests / 7985965 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7985965 pgsql_mnesia_24 / pgsql_mnesia / 7985965 internal_mnesia_25 / internal_mnesia / 7985965 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 7985965 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 7985965 service_mongoose_system_metrics_SUITE:system_metrics_are_reported_to_configurable_google_analytics{error,
{{assertEqual,
[{module,service_mongoose_system_metrics_SUITE},
{line,470},
{expression,"ActualTrackingIds"},
{expected,[<<"UA-151671255-1">>,<<"UA-EXTRA-TRACKING-ID">>]},
{value,[<<"UA-151671255-1">>]}]},
[{service_mongoose_system_metrics_SUITE,
events_are_reported_to_tracking_ids,1,
[{file,
"/home/circleci/project/big_tests/tests/service_mongoose_system_metrics_SUITE.erl"},
{line,470}]},
{service_mongoose_system_metrics_SUITE,
system_metrics_are_reported_to_configurable_google_analytics,1,
[{file,
"/home/circleci/project/big_tests/tests/service_mongoose_system_metrics_SUITE.erl"},
{line,204}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} dynamic_domains_mysql_redis_25 / mysql_redis / 7985965 pgsql_mnesia_25 / pgsql_mnesia / 7985965 mysql_redis_25 / mysql_redis / 7985965 metrics_session_SUITE:session_global:session_global{error,
{{totalSessionCount,
{value,1},
[{times,25,
{error,
{badmatch,{value,0}},
[{metrics_helper,assert_counter,3,
[{file,
"/home/circleci/project/big_tests/tests/metrics_helper.erl"},
{line,36}]},
{mongoose_helper,do_wait_until,2,
[{file,
"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,374}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,
[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}]},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,371}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 7985965 mssql_mnesia_25 / odbc_mssql_mnesia / 7985965 service_domain_db_SUITE:db:db_get_all_static{error,
{{badrpc,{'EXIT',{timeout,{gen_server,call,[service_domain_db,ping]}}}},
[{distributed_helper,rpc,
[#{node => mongooseim@localhost},service_domain_db,sync_local,[]],
[{file,
"/home/circleci/project/big_tests/tests/distributed_helper.erl"},
{line,117}]},
{service_domain_db_SUITE,sync_local,1,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,1205}]},
{service_domain_db_SUITE,sync,0,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,1184}]},
{service_domain_db_SUITE,db_get_all_static,1,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,356}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} service_domain_db_SUITE:db:db_get_all_dynamic{error,
{{badrpc,timeout},
[{distributed_helper,rpc,
[#{node => mongooseim@localhost},service_domain_db,sync_local,[]],
[{file,
"/home/circleci/project/big_tests/tests/distributed_helper.erl"},
{line,117}]},
{service_domain_db_SUITE,sync_local,1,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,1205}]},
{service_domain_db_SUITE,sync,0,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,1184}]},
{service_domain_db_SUITE,db_get_all_dynamic,1,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,366}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} service_domain_db_SUITE:db:db_out_of_sync_restarts_service{error,
{{badrpc,timeout},
[{distributed_helper,rpc,
[#{node => ejabberd2@localhost},service_domain_db,sync_local,[]],
[{file,
"/home/circleci/project/big_tests/tests/distributed_helper.erl"},
{line,117}]},
{service_domain_db_SUITE,sync_local,1,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,1205}]},
{service_domain_db_SUITE,db_out_of_sync_restarts_service,1,
[{file,
"/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
{line,634}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} riak_mnesia_24 / riak_mnesia / 7985965 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
There are random CI failures, but the docker image would not build anyway (failing small tests as expected).
No description provided.