-
Notifications
You must be signed in to change notification settings - Fork 428
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
Check push form fields #3916
Check push form fields #3916
Conversation
Codecov ReportBase: 83.13% // Head: 83.13% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #3916 +/- ##
=======================================
Coverage 83.13% 83.13%
=======================================
Files 535 535
Lines 34109 34109
=======================================
Hits 28358 28358
Misses 5751 5751 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. |
This comment was marked as outdated.
This comment was marked as outdated.
e421d8f
to
df0471f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Add a test to check this condition.
df0471f
to
13eda68
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Previously missing keys or values would be reported as 'undefined', which could result in a crash (Mnesia) or would be silently converted to binaries (RDBMS).
13eda68
to
9e15599
Compare
small_tests_24 / small_tests / 9e15599 small_tests_25 / small_tests / 9e15599 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 9e15599 ldap_mnesia_24 / ldap_mnesia / 9e15599 ldap_mnesia_25 / ldap_mnesia / 9e15599 dynamic_domains_mysql_redis_25 / mysql_redis / 9e15599 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 9e15599 pgsql_mnesia_24 / pgsql_mnesia / 9e15599 internal_mnesia_25 / internal_mnesia / 9e15599 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 9e15599 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 9e15599 riak_mnesia_24 / riak_mnesia / 9e15599 pgsql_mnesia_25 / pgsql_mnesia / 9e15599 mysql_redis_25 / mysql_redis / 9e15599 mssql_mnesia_25 / odbc_mssql_mnesia / 9e15599 pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,
{{badmatch,
[{xmlel,<<"message">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2710@localhost">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2710@localhost/res1">>},
{<<"type">>,<<"headline">>}],
[{xmlel,<<"event">>,
[{<<"xmlns">>,
<<"http://jabber.org/protocol/pubsub#event">>}],
[{xmlel,<<"items">>,
[{<<"node">>,<<"n2LTTr68n9gNBCtpiY3WlA==">>}],
[{xmlel,<<"item">>,
[{<<"id">>,<<"salmon">>}],
[{xmlel,<<"entry">>,
[{<<"xmlns">>,
<<"http://www.w3.org/2005/Atom">>}],
[]}]}]}]},
{xmlel,<<"headers">>,
[{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
[]}]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{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}]}]}} |
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
When the push notification node is enabled with an invalid form (especially with missing keys or values), it could still be saved to the DB. Afterwards, sending a push notification either resulted in
undefined
values being sent (RDBMS) or the a crash (Mnesia). This PR fixes this issue by more strict checking of the form values and rejecting invalid forms.This PR resolves the most likely cause of #3806