-
Notifications
You must be signed in to change notification settings - Fork 421
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
Safely parse mam retractions #3513
Conversation
With the current code, a malicious retraction could have a namespace but then not declare any id, and in that case the function would return `{origin_id | stanza_id, none}`, which breaks the contract that the second term must be a binary. In a worst-case scenario, this could trigger an exception later on, maybe even killing one of the MAM async workers with their message queues if the exception is not caught.
Codecov Report
@@ Coverage Diff @@
## master #3513 +/- ##
=======================================
Coverage 81.02% 81.02%
=======================================
Files 419 419
Lines 32309 32310 +1
=======================================
+ Hits 26177 26179 +2
+ Misses 6132 6131 -1
Continue to review full report at Codecov.
|
small_tests_24 / small_tests / 590ac45 small_tests_23 / small_tests / 590ac45 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 590ac45 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 590ac45 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 590ac45 dynamic_domains_mysql_redis_24 / mysql_redis / 590ac45 ldap_mnesia_23 / ldap_mnesia / 590ac45 ldap_mnesia_24 / ldap_mnesia / 590ac45 internal_mnesia_24 / internal_mnesia / 590ac45 pgsql_mnesia_23 / pgsql_mnesia / 590ac45 pgsql_mnesia_24 / pgsql_mnesia / 590ac45 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 590ac45 mssql_mnesia_24 / odbc_mssql_mnesia / 590ac45 mysql_redis_24 / mysql_redis / 590ac45 riak_mnesia_24 / riak_mnesia / 590ac45 |
small_tests_24 / small_tests / 4898ed3 small_tests_23 / small_tests / 4898ed3 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 4898ed3 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4898ed3 mam_SUITE:rdbms_async_pool_mam_all:archived:metrics_incremented_for_async_pools{error,{test_case_failed,"ASSERT EQUAL\n\tExpected false\n\tValue true\n"}} dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 4898ed3 dynamic_domains_mysql_redis_24 / mysql_redis / 4898ed3 internal_mnesia_24 / internal_mnesia / 4898ed3 ldap_mnesia_23 / ldap_mnesia / 4898ed3 ldap_mnesia_24 / ldap_mnesia / 4898ed3 mysql_redis_24 / mysql_redis / 4898ed3 pgsql_mnesia_24 / pgsql_mnesia / 4898ed3 pgsql_mnesia_23 / pgsql_mnesia / 4898ed3 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 4898ed3 riak_mnesia_24 / riak_mnesia / 4898ed3 mssql_mnesia_24 / odbc_mssql_mnesia / 4898ed3 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4898ed3 |
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 nice, also some test improvements 👍
With the current code, a malicious retraction could have a namespace but
then not declare any id, and in that case the function would return
{origin_id | stanza_id, none}
, which breaks the contract that thesecond term must be a binary. In a worst-case scenario, this could
trigger an exception later on, maybe even killing one of the MAM
async workers with their message queues if the exception is not caught.