Identify only stored users on offline routing #3395
Conversation
When we arrive at this point, it is because the user we're sending a message to has not been found to be online, so here the routing mech needs to verify if the user actually exists, to know if it should go the 'offline_message_hook' or the 'service_unavailable' way. But, it would actually be incorrect to verify for anonymous users at this point: first, we have already verified that the receiver is not online, and anonymous accounts are ephemeral, they only exist as long as the user is online, so checking for anonymous accounts is unnecessary and redundant. To make things worse, there's then a race condition here, where the anonymous account that wasn't connected before, is connected by the time we check again, and then this code takes the wrong path! This fix not only curates all that Anonymous mess, but also gives a chance to use the users cache at this point more opportunistically.
Codecov Report
@@ Coverage Diff @@
## master #3395 +/- ##
==========================================
+ Coverage 80.69% 80.74% +0.04%
==========================================
Files 397 397
Lines 32199 32200 +1
==========================================
+ Hits 25984 26000 +16
+ Misses 6215 6200 -15
Continue to review full report at Codecov.
|
|
small_tests_24 / small_tests / 8196b28 internal_mnesia_24 / internal_mnesia / 8196b28 small_tests_23 / small_tests / 8196b28 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 8196b28 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 8196b28 domain_removal_SUITE:last_removal:last_removal{error,
{test_case_failed,
{has_stanzas_but_shouldnt,
{client,
<<"alicE_last_removal_31.201594@domain.example.com/res1">>,
escalus_tcp,<0.2296.2>,
[{event_manager,<0.2292.2>},
{server,<<"domain.example.com">>},
{username,<<"alicE_last_removal_31.201594">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.2292.2>},
{server,<<"domain.example.com">>},
{username,<<"alicE_last_removal_31.201594">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"alicE_last_removal_31.201594">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"alicE_last_removal_31.201594">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"4a45549e26109e90">>}]},
[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_last_removal_31.201594@domain.example.com/res1">>},
{<<"to">>,
<<"alice_last_removal_31.201594@domain.example.com/res1">>},
{<<"xml:lang">>,<<"en">>}],
[]}]}}}dynamic_domains_mysql_redis_24 / mysql_redis / 8196b28 mam_SUITE:rdbms_simple_prefs_cases:messages_filtered_when_prefs_default_policy_is_never{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok,ok]\n"}}dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 8196b28 ldap_mnesia_24 / ldap_mnesia / 8196b28 ldap_mnesia_23 / ldap_mnesia / 8196b28 pgsql_mnesia_24 / pgsql_mnesia / 8196b28 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 8196b28 mysql_redis_24 / mysql_redis / 8196b28 mam_SUITE:rdbms_cache_prefs_cases:messages_filtered_when_prefs_default_policy_is_never{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok]\n"}}pgsql_mnesia_23 / pgsql_mnesia / 8196b28 mssql_mnesia_24 / odbc_mssql_mnesia / 8196b28 riak_mnesia_24 / riak_mnesia / 8196b28 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 8196b28 |
When we arrive at this point, it is because the user we're sending a
message to has not been found to be online, so here the routing mech
needs to verify if the user actually exists, to know if it should go
the 'offline_message_hook' or the 'service_unavailable' way. But, it
would actually be incorrect to verify for anonymous users at this point:
first, we have already verified that the receiver is not online, and
anonymous accounts are ephemeral, they only exist as long as the user is
online, so checking for anonymous accounts is unnecessary and redundant.
To make things worse, there's then a race condition here, where the
anonymous account that wasn't connected before, is connected by the time
we check again, and then this code takes the wrong path!
This fix not only curates all that Anonymous mess, but also gives a
chance to use the users cache at this point more opportunistically.