Pubsub bugs

Hi, I found several bugs in module pubsub.
1.
In the function get_items/6 in mod_pubsub.erl:

get_items(Host, Node, From, SubId, SMaxItems, ItemIds) ->
    MaxItems =
        if
            SMaxItems == "" -> get_max_items_node(Host);
           true ->
                case catch list_to_integer(SMaxItems) of
                    {'EXIT', _} -> {error, 'bad-request'};
                    Val -> Val
                end
        end,
    ……

I find no matter SMaxItems has a value or not, it's binary type, the match clause: SMaxItem == "" will always be false, and the call to list_to_integer(SMaxItems) obviously will return {'EXIT', badarg} and finally ejabberd will return a bad-request error to client.

2.
I tried to get node subscription options from pubsub service in ejabberd, but I got this error:

=ERROR REPORT==== 19-Jul-2011::12:45:29 ===
E(<0.567.0>:mod_pubsub:4386) : transaction return internal error: {'EXIT',
                                                                   {function_clause,
                                                                    [{node_flat,
                                                                      get_subscriptions,
                                                                      [2,
                                                                       {<<"test">>,
                                                                        <<"localhost">>,
                                                                        undefined}]},
                                                                     {mod_pubsub,
                                                                      node_call,
                                                                      3},
                                                                     {mod_pubsub,
                                                                      get_options_helper,
                                                                      6}]}}

I think this error is because node_flat:get_subscriptions/2 requirs record jid argument, but the call just give it a {<<"test">>,<<"localhost">>,undefined} tuple.

If someone can confirm these bugs, I'll be appreciated.

Best regards,

Hi, everyone, I posted this

Hi, everyone, I posted this message serveral days ago, but nobody answers me, so I modified the source code myself, and ejabberd can work now, here are my modifications:

diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl
index fc50a1c..c2432c6 100644
--- a/src/mod_pubsub/mod_pubsub.erl
+++ b/src/mod_pubsub/mod_pubsub.erl
@@ -2639,9 +2639,9 @@ purge_node(Host, Node, Owner) ->
get_items(Host, Node, From, SubId, SMaxItems, ItemIds) ->
     MaxItems =
        if
-           SMaxItems == "" -> get_max_items_node(Host);
+           SMaxItems == <<"">> -> get_max_items_node(Host);
            true ->
-               case catch list_to_integer(SMaxItems) of
+               case catch list_to_integer(binary_to_list(SMaxItems)) of
                    {'EXIT', _} -> {error, 'bad-request'};
                    Val -> Val
                end
@@ -2957,12 +2957,13 @@ get_options(Host, Node, JID, SubId, Lang) ->
     end.

get_options_helper(JID, Lang, Node, Nidx, SubId, Type) ->
-    Subscriber = try exmpp_jid:parse(JID) of
+    {U, S, R} = try exmpp_jid:parse(JID) of
                     J -> jlib:short_jid(J)
                 catch
                     _ ->
                         exmpp_jid:make("", "", "") %% TODO, check if use <<>> instead of ""
                 end,
+    Subscriber = #jid{node = U, domain = S, resource = R},
     {result, Subs} = node_call(Type, get_subscriptions,
                               [Nidx, Subscriber]),
     SubIds = lists:foldl(fun({subscribed, SID}, Acc) ->

After that, I tried some other features in pubsub module, and now, I find another bug, here it is:
User m9@localhost is trying to subscribe to a node owned by test@localhost, and m9 is in test's roster group "stranger". The acces model of the node is "roster", and the allowed roster groups contain group "stranger", but the server finally gives the error "not-in-roster-group".

After tracking this problem, I find the bug is in function get_roster_info/4:

mod_pubsub.erl:

get_roster_info(OwnerUser, OwnerServer, {SubscriberUser, SubscriberServer, _}, AllowedGroups) ->
    {Subscription, Groups} =
ejabberd_hooks:run_fold(
  roster_get_jid_info, OwnerServer,
  {none, []},
  [OwnerUser, OwnerServer, exmpp_jid:make({SubscriberUser, SubscriberServer, undefined})]),
    PresenceSubscription = (Subscription == both) orelse (Subscription == from)
orelse ({OwnerUser, OwnerServer} == {SubscriberUser, SubscriberServer}),
    RosterGroup = lists:any(fun(Group) ->
    lists:member(Group, AllowedGroups)
    end, Groups),
    {PresenceSubscription, RosterGroup};

When calling lists:member(Group, AllowedGroups), the context is like this:

< AllowedGroups = ["stranger","friend","family"]
< Owners = [{<<"test">>,<<"localhost">>,undefined}]
< Groups = [<<"stranger">>]
< AllowedGroups = ["stranger","friend","family"]

We can see the type of Group doesn't match the type of elements in list AllowedGroups, so the return value will always be false. The bug is obvious, and my concern is from when and from where, those bugs are brought in, and there may be other bugs similar to this, I really hope someone could give me a reply. please, any comments or suggestions are greatly appreciated.

Syndicate content