Merge pull request #3916 from esl/check-push-form-fields

Check push form fields
esl · Jan 4, 2023 · 459c5ea · 459c5ea
2 parents 13c4ae9 + 9e15599
commit 459c5ea
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 10 deletions.
diff --git a/big_tests/tests/push_SUITE.erl b/big_tests/tests/push_SUITE.erl
@@ -263,6 +263,12 @@ enable_should_fail_with_invalid_attributes(Config) ->
             escalus:send(Bob, enable_stanza(PubsubJID, <<>>)),
             escalus:assert(is_error, [<<"modify">>, <<"bad-request">>],
                            escalus:wait_for_stanza(Bob)),
+
+            %% Missing value
+            escalus:send(Bob, enable_stanza(PubsubJID, <<"nodeId">>,
+                                            [{<<"secret1">>, undefined}])),
+            escalus:assert(is_error, [<<"modify">>, <<"bad-request">>],
+                           escalus:wait_for_stanza(Bob)),
             ok
         end).
 

diff --git a/big_tests/tests/push_helper.erl b/big_tests/tests/push_helper.erl
@@ -60,6 +60,8 @@ make_form(Fields) ->
     #xmlel{name = <<"x">>, attrs = [{<<"xmlns">>, ?NS_XDATA}, {<<"type">>, <<"submit">>}],
            children = [make_form_field(Name, Value) || {Name, Value} <- Fields]}.
 
+make_form_field(Name, undefined) ->
+    #xmlel{name = <<"field">>, attrs = [{<<"var">>, Name}]};
 make_form_field(Name, Value) ->
     #xmlel{name = <<"field">>,
            attrs = [{<<"var">>, Name}],

diff --git a/src/event_pusher/mod_event_pusher_push.erl b/src/event_pusher/mod_event_pusher_push.erl
@@ -231,25 +231,39 @@ parse_request(_) ->
 parse_form(undefined) ->
     [];
 parse_form(Form) ->
+    case is_valid_form(Form) of
+        true ->
+            parse_form_fields(Form);
+        false ->
+            invalid_form
+    end.
+
+-spec is_valid_form(exml:element()) -> boolean().
+is_valid_form(Form) ->
     IsForm = ?NS_XDATA == exml_query:attr(Form, <<"xmlns">>),
     IsSubmit = <<"submit">> == exml_query:attr(Form, <<"type">>, <<"submit">>),
+    IsForm andalso IsSubmit.
 
+-spec parse_form_fields(exml:element()) -> invalid_form | form().
+parse_form_fields(Form) ->
     FieldsXML = exml_query:subelements(Form, <<"field">>),
     Fields = [{exml_query:attr(Field, <<"var">>),
                exml_query:path(Field, [{element, <<"value">>}, cdata])} || Field <- FieldsXML],
-    {[{_, FormType}], CustomFields} = lists:partition(
-                                        fun({Name, _}) ->
-                                                Name == <<"FORM_TYPE">>
-                                        end, Fields),
-    IsFormTypeCorrect = ?NS_PUBSUB_PUB_OPTIONS == FormType,
-
-    case IsForm andalso IsSubmit andalso IsFormTypeCorrect of
-        true ->
-            CustomFields;
-        false ->
+    case lists:keytake(<<"FORM_TYPE">>, 1, Fields) of
+        {value, {_, ?NS_PUBSUB_PUB_OPTIONS}, CustomFields} ->
+            case are_form_fields_valid(CustomFields) of
+                true ->
+                    CustomFields;
+                false ->
+                    invalid_form
+            end;
+        _ ->
             invalid_form
     end.
 
+are_form_fields_valid(Fields) ->
+    lists:all(fun({Key, Value}) -> is_binary(Key) andalso is_binary(Value) end, Fields).
+
 -spec enable_node(mongooseim:host_type(), jid:jid(), jid:jid(), pubsub_node(), form()) ->
     ok | {error, Reason :: term()}.
 enable_node(HostType, From, BarePubSubJID, Node, FormFields) ->