From b2bd51d3e3f4f1e9991cfc9a3d9a5623d84f5f9d Mon Sep 17 00:00:00 2001 From: Christophe Romain Date: Tue, 27 Oct 2009 14:24:33 +0000 Subject: [PATCH] fix disco bugs, thanks to Brian Cully (EJAB-1088) SVN Revision: 2712 --- src/mod_pubsub/mod_pubsub.erl | 27 +++++---- src/mod_pubsub/mod_pubsub_odbc.erl | 33 ++++++----- src/mod_pubsub/pubsub_odbc.patch | 94 ++++++++++++++---------------- 3 files changed, 82 insertions(+), 72 deletions(-) diff --git a/src/mod_pubsub/mod_pubsub.erl b/src/mod_pubsub/mod_pubsub.erl index cce677b29..d2af36468 100644 --- a/src/mod_pubsub/mod_pubsub.erl +++ b/src/mod_pubsub/mod_pubsub.erl @@ -1123,13 +1123,18 @@ iq_disco_info(Host, SNode, From, Lang) -> end. iq_disco_items(Host, [], From) -> - {result, lists:map( - fun(#pubsub_node{nodeid = {_, SubNode}, type = Type}) -> - {result, Path} = node_call(Type, node_to_path, [SubNode]), - [Name | _] = lists:reverse(Path), - #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), - ?XMLATTR('name', Name) | nodeAttr(SubNode)]} - end, tree_action(Host, get_subnodes, [Host, [], From]))}; + case tree_action(Host, get_subnodes, [Host, <<>>, From]) of + Nodes when is_list(Nodes) -> + {result, lists:map( + fun(#pubsub_node{nodeid = {_, SubNode}, type = Type}) -> + {result, Path} = node_call(Type, node_to_path, [SubNode]), + [Name | _] = lists:reverse(Path), + #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), + ?XMLATTR('name', Name) | nodeAttr(SubNode)]} + end, Nodes)}; + Other -> + Other + end; iq_disco_items(Host, Item, From) -> case string:tokens(Item, "!") of [_SNode, _ItemID] -> @@ -1145,7 +1150,9 @@ iq_disco_items(Host, Item, From) -> end, Nodes = lists:map( fun(#pubsub_node{nodeid = {_, SubNode}}) -> - #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host) | nodeAttr(SubNode)]} + {result, Path} = node_call(Type, node_to_path, [SubNode]), + [Name|_] = lists:reverse(Path), + #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), ?XMLATTR('name', Name) | nodeAttr(SubNode)]} end, tree_call(Host, get_subnodes, [Host, Node, From])), Items = lists:map( fun(#pubsub_item{itemid = {RN, _}}) -> @@ -3136,10 +3143,10 @@ broadcast_stanza(Host, Node, _NodeId, _Type, NodeOptions, SubsByDepth, NotifyTyp case Host of {LUser, LServer, LResource} -> SenderResource = case LResource of - [] -> + undefined -> case user_resources(LUser, LServer) of [Resource|_] -> Resource; - _ -> "" + _ -> <<"">> end; _ -> LResource diff --git a/src/mod_pubsub/mod_pubsub_odbc.erl b/src/mod_pubsub/mod_pubsub_odbc.erl index 92e7275ea..1ca07a766 100644 --- a/src/mod_pubsub/mod_pubsub_odbc.erl +++ b/src/mod_pubsub/mod_pubsub_odbc.erl @@ -922,21 +922,26 @@ iq_disco_info(Host, SNode, From, Lang) -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s)]}, #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_VCARD_s)]}] ++ lists:map(fun - ("rsm") -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_RSM_s)]}; - (Feature) -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s++"#"++Feature)]} + ("rsm") -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_RSM_s)]}; + (Feature) -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s++"#"++Feature)]} end, features(Host, Node))}; _ -> node_disco_info(Host, Node, From) end. -iq_disco_items(Host, [], From, _RSM) -> - {result, lists:map( - fun(#pubsub_node{nodeid = {_, SubNode}, type = Type}) -> - {result, Path} = node_call(Type, node_path, [SubNode]), - [Name | _] = lists:reverse(Path), - #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), - ?XMLATTR('name', Name) | nodeAttr(SubNode)]} - end, tree_action(Host, get_subnodes, [Host, <<>>, From]))}; +iq_disco_items(Host, [], Fromi, _RSM) -> + case tree_action(Host, get_subnodes, [Host, <<>>, From]) of + Nodes when is_list(Nodes) -> + {result, lists:map( + fun(#pubsub_node{nodeid = {_, SubNode}, type = Type}) -> + {result, Path} = node_call(Type, node_to_path, [SubNode]), + [Name | _] = lists:reverse(Path), + #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), + ?XMLATTR('name', Name) | nodeAttr(SubNode)]} + end, Nodes)}; + Other -> + Other + end; iq_disco_items(Host, Item, From, RSM) -> case string:tokens(Item, "!") of [_SNode, _ItemID] -> @@ -952,7 +957,9 @@ iq_disco_items(Host, Item, From, RSM) -> end, Nodes = lists:map( fun(#pubsub_node{nodeid = {_, SubNode}}) -> - #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host) | nodeAttr(SubNode)]} + {result, Path} = node_call(Type, node_to_path, [SubNode]), + [Name|_] = lists:reverse(Path), + #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), ?XMLATTR('name', Name) | nodeAttr(SubNode)]} end, tree_call(Host, get_subnodes, [Host, Node, From])), Items = lists:map( fun(#pubsub_item{itemid = {RN, _}}) -> @@ -2946,10 +2953,10 @@ broadcast_stanza(Host, Node, _NodeId, _Type, NodeOptions, SubsByDepth, NotifyTyp case Host of {LUser, LServer, LResource} -> SenderResource = case LResource of - [] -> + undefined -> case user_resources(LUser, LServer) of [Resource|_] -> Resource; - _ -> "" + _ -> <<"">> end; _ -> LResource diff --git a/src/mod_pubsub/pubsub_odbc.patch b/src/mod_pubsub/pubsub_odbc.patch index 0580f7592..ec516a168 100644 --- a/src/mod_pubsub/pubsub_odbc.patch +++ b/src/mod_pubsub/pubsub_odbc.patch @@ -1,5 +1,5 @@ ---- mod_pubsub.erl 2009-10-21 10:28:28.000000000 +0200 -+++ mod_pubsub_odbc.erl 2009-10-21 10:28:28.000000000 +0200 +--- mod_pubsub.erl 2009-10-27 15:19:20.000000000 +0100 ++++ mod_pubsub_odbc.erl 2009-10-27 15:21:49.000000000 +0100 @@ -42,7 +42,7 @@ %%% 6.2.3.1, 6.2.3.5, and 6.3. For information on subscription leases see %%% XEP-0060 section 12.18. @@ -343,39 +343,35 @@ end, features(Type))] end, %% TODO: add meta-data info (spec section 5.4) -@@ -1115,22 +921,23 @@ +@@ -1115,14 +921,15 @@ #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_DISCO_ITEMS_s)]}, #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s)]}, #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_VCARD_s)]}] ++ - lists:map(fun(Feature) -> - #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s++"#"++Feature)]} + lists:map(fun -+ ("rsm") -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_RSM_s)]}; -+ (Feature) -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s++"#"++Feature)]} ++ ("rsm") -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_RSM_s)]}; ++ (Feature) -> #xmlel{ns = ?NS_DISCO_INFO, name = 'feature', attrs = [?XMLATTR('var', ?NS_PUBSUB_s++"#"++Feature)]} end, features(Host, Node))}; _ -> node_disco_info(Host, Node, From) end. -iq_disco_items(Host, [], From) -> -+iq_disco_items(Host, [], From, _RSM) -> - {result, lists:map( - fun(#pubsub_node{nodeid = {_, SubNode}, type = Type}) -> -- {result, Path} = node_call(Type, node_to_path, [SubNode]), -- [Name | _] = lists:reverse(Path), -+ {result, Path} = node_call(Type, node_path, [SubNode]), -+ [Name | _] = lists:reverse(Path), - #xmlel{ns = ?NS_DISCO_ITEMS, name = 'item', attrs = [?XMLATTR('jid', Host), -- ?XMLATTR('name', Name) | nodeAttr(SubNode)]} -- end, tree_action(Host, get_subnodes, [Host, [], From]))}; ++iq_disco_items(Host, [], Fromi, _RSM) -> + case tree_action(Host, get_subnodes, [Host, <<>>, From]) of + Nodes when is_list(Nodes) -> + {result, lists:map( +@@ -1135,7 +942,7 @@ + Other -> + Other + end; -iq_disco_items(Host, Item, From) -> -+ ?XMLATTR('name', Name) | nodeAttr(SubNode)]} -+ end, tree_action(Host, get_subnodes, [Host, <<>>, From]))}; +iq_disco_items(Host, Item, From, RSM) -> case string:tokens(Item, "!") of [_SNode, _ItemID] -> {result, []}; -@@ -1138,10 +945,10 @@ +@@ -1143,10 +950,10 @@ Node = string_to_node(SNode), Action = fun(#pubsub_node{type = Type, id = NodeId}) -> @@ -389,7 +385,7 @@ end, Nodes = lists:map( fun(#pubsub_node{nodeid = {_, SubNode}}) -> -@@ -1150,9 +957,10 @@ +@@ -1157,9 +964,10 @@ Items = lists:map( fun(#pubsub_item{itemid = {RN, _}}) -> {result, Name} = node_call(Type, get_item_name, [Host, Node, RN]), @@ -402,7 +398,7 @@ end, case transaction(Host, Node, Action, sync_dirty) of {result, {_, Result}} -> {result, Result}; -@@ -1208,7 +1016,7 @@ +@@ -1215,7 +1023,7 @@ iq_pubsub(Host, ServerHost, From, IQType, SubEl, Lang, Access, Plugins) -> case exmpp_xml:remove_cdata_from_list(SubEl#xmlel.children) of [#xmlel{name = Name, attrs = Attrs, children = Els} | Rest] -> @@ -411,7 +407,7 @@ case {IQType, Name} of {set, 'create'} -> Config = case Rest of -@@ -1283,7 +1091,8 @@ +@@ -1290,7 +1098,8 @@ (_, Acc) -> Acc end, [], exmpp_xml:remove_cdata_from_list(Els)), @@ -421,7 +417,7 @@ {get, 'subscriptions'} -> get_subscriptions(Host, Node, From, Plugins); {get, 'affiliations'} -> -@@ -1305,8 +1114,9 @@ +@@ -1312,8 +1121,9 @@ end. iq_pubsub_owner(Host, ServerHost, From, IQType, SubEl, Lang) -> @@ -433,7 +429,7 @@ case Action of [#xmlel{name = Name, attrs = Attrs, children = Els}] -> Node = string_to_node(exmpp_xml:get_attribute_from_list_as_list(Attrs, 'node', "")), -@@ -1433,7 +1243,8 @@ +@@ -1440,7 +1250,8 @@ _ -> [] end end, @@ -443,7 +439,7 @@ sync_dirty) of {result, Res} -> Res; Err -> Err -@@ -1478,7 +1289,7 @@ +@@ -1485,7 +1296,7 @@ %%% authorization handling @@ -452,7 +448,7 @@ Lang = "en", %% TODO fix {U, S, R} = Subscriber, Stanza = #xmlel{ns = ?NS_JABBER_CLIENT, name = 'message', children = -@@ -1508,7 +1319,7 @@ +@@ -1515,7 +1326,7 @@ lists:foreach(fun(Owner) -> {U, S, R} = Owner, ejabberd_router ! {route, service_jid(Host), exmpp_jid:make(U, S, R), Stanza} @@ -461,7 +457,7 @@ find_authorization_response(Packet) -> Els = Packet#xmlel.children, -@@ -1550,7 +1361,7 @@ +@@ -1557,7 +1368,7 @@ end, Stanza = event_stanza( [#xmlel{ns = ?NS_PUBSUB_EVENT, name = 'subscription', attrs = @@ -470,7 +466,7 @@ }]), ejabberd_router ! {route, service_jid(Host), JID, Stanza}. -@@ -1561,14 +1372,14 @@ +@@ -1568,14 +1379,14 @@ {{value, {_, [SNode]}}, {value, {_, [SSubscriber]}}, {value, {_, [SAllow]}}} -> Node = string_to_node(SNode), @@ -488,7 +484,7 @@ {result, Subscriptions} = node_call(Type, get_subscriptions, [NodeId, Subscriber]), if not IsApprover -> -@@ -1763,7 +1574,7 @@ +@@ -1770,7 +1581,7 @@ end, Reply = #xmlel{ns = ?NS_PUBSUB, name = 'pubsub', children = [#xmlel{ns = ?NS_PUBSUB, name = 'create', attrs = nodeAttr(Node)}]}, @@ -497,7 +493,7 @@ {result, {Result, broadcast}} -> %%Lang = "en", %% TODO: fix %%OwnerKey = jlib:jid_tolower(jlib:jid_remove_resource(Owner)), -@@ -1872,7 +1683,7 @@ +@@ -1879,7 +1690,7 @@ %%
  • The node does not exist.
    • %% subscribe_node(Host, Node, From, JID, Configuration) -> @@ -506,7 +502,7 @@ {result, GoodSubOpts} -> GoodSubOpts; _ -> invalid end, -@@ -1883,7 +1694,7 @@ +@@ -1890,7 +1701,7 @@ {undefined, undefined, undefined} end, SubId = uniqid(), @@ -515,7 +511,7 @@ Features = features(Type), SubscribeFeature = lists:member("subscribe", Features), OptionsFeature = lists:member("subscription-options", Features), -@@ -1902,9 +1713,13 @@ +@@ -1909,9 +1720,13 @@ {"", "", ""} -> {false, false}; _ -> @@ -532,7 +528,7 @@ end end, if -@@ -2237,7 +2052,7 @@ +@@ -2244,7 +2059,7 @@ %%

    The permission are not checked in this function.

    %% @todo We probably need to check that the user doing the query has the right %% to read the items. @@ -541,7 +537,7 @@ MaxItems = if SMaxItems == "" -> get_max_items_node(Host); -@@ -2276,11 +2091,11 @@ +@@ -2283,11 +2098,11 @@ node_call(Type, get_items, [NodeId, From, AccessModel, PresenceSubscription, RosterGroup, @@ -555,7 +551,7 @@ SendItems = case ItemIDs of [] -> Items; -@@ -2293,7 +2108,7 @@ +@@ -2300,7 +2115,7 @@ %% number of items sent to MaxItems: {result, #xmlel{ns = ?NS_PUBSUB, name = 'pubsub', children = [#xmlel{ns = ?NS_PUBSUB, name = 'items', attrs = nodeAttr(Node), children = @@ -564,7 +560,7 @@ Error -> Error end -@@ -2325,17 +2140,29 @@ +@@ -2332,17 +2147,29 @@ %% @doc

    Resend the items of a node to the user.

    %% @todo use cache-last-item feature send_items(Host, Node, NodeId, Type, LJID, last) -> @@ -601,7 +597,7 @@ send_items(Host, Node, NodeId, Type, {LU, LS, LR} = LJID, Number) -> ToSend = case node_action(Host, Type, get_items, [NodeId, LJID]) of {result, []} -> -@@ -2464,29 +2291,12 @@ +@@ -2471,29 +2298,12 @@ error -> {error, 'bad-request'}; _ -> @@ -634,7 +630,7 @@ end, Entities), {result, []}; _ -> -@@ -2541,11 +2351,11 @@ +@@ -2548,11 +2358,11 @@ end. read_sub(Subscriber, Node, NodeID, SubID, Lang) -> @@ -648,7 +644,7 @@ OptionsEl = #xmlel{ns = ?NS_PUBSUB, name = 'options', attrs = [ ?XMLATTR('jid', exmpp_jid:to_binary(Subscriber)), ?XMLATTR('Subid', SubID) | nodeAttr(Node)], -@@ -2572,7 +2382,7 @@ +@@ -2579,7 +2389,7 @@ end. set_options_helper(Configuration, JID, NodeID, SubID, Type) -> @@ -657,7 +653,7 @@ {result, GoodSubOpts} -> GoodSubOpts; _ -> invalid end, -@@ -2602,7 +2412,7 @@ +@@ -2609,7 +2419,7 @@ write_sub(_Subscriber, _NodeID, _SubID, invalid) -> {error, extended_error('bad-request', "invalid-options")}; write_sub(Subscriber, NodeID, SubID, Options) -> @@ -666,7 +662,7 @@ {error, notfound} -> {error, extended_error('not-acceptable', "invalid-subid")}; {result, _} -> -@@ -2775,8 +2585,8 @@ +@@ -2782,8 +2592,8 @@ ?XMLATTR('subsription', subscription_to_string(Sub)) | nodeAttr(Node)]}]}]}, ejabberd_router ! {route, service_jid(Host), JID, Stanza} end, @@ -677,7 +673,7 @@ true -> Result = lists:foldl(fun({JID, Subscription, SubId}, Acc) -> -@@ -3066,7 +2876,7 @@ +@@ -3073,7 +2883,7 @@ {Depth, [{N, get_node_subs(N)} || N <- Nodes]} end, tree_call(Host, get_parentnodes_tree, [Host, Node, service_jid(Host)]))} end, @@ -686,7 +682,7 @@ {result, CollSubs} -> CollSubs; _ -> [] end. -@@ -3080,9 +2890,9 @@ +@@ -3087,9 +2897,9 @@ get_options_for_subs(NodeID, Subs) -> lists:foldl(fun({JID, subscribed, SubID}, Acc) -> @@ -698,7 +694,7 @@ _ -> Acc end; (_, Acc) -> -@@ -3090,7 +2900,7 @@ +@@ -3097,7 +2907,7 @@ end, [], Subs). % TODO: merge broadcast code that way @@ -707,7 +703,7 @@ %broadcast(Host, Node, NodeId, Type, NodeOptions, Feature, Force, ElName, SubEls) -> % case (get_option(NodeOptions, Feature) or Force) of % true -> -@@ -3282,6 +3092,30 @@ +@@ -3289,6 +3099,30 @@ Result end. @@ -738,7 +734,7 @@ %% @spec (Host, Options) -> MaxItems %% Host = host() %% Options = [Option] -@@ -3671,7 +3505,13 @@ +@@ -3678,7 +3512,13 @@ tree_action(Host, Function, Args) -> ?DEBUG("tree_action ~p ~p ~p",[Host,Function,Args]), Fun = fun() -> tree_call(Host, Function, Args) end, @@ -753,7 +749,7 @@ %% @doc

    node plugin call.

    node_call(Type, Function, Args) -> -@@ -3691,13 +3531,13 @@ +@@ -3698,13 +3538,13 @@ node_action(Host, Type, Function, Args) -> ?DEBUG("node_action ~p ~p ~p ~p",[Host,Type,Function,Args]), @@ -769,7 +765,7 @@ case tree_call(Host, get_node, [Host, Node]) of N when is_record(N, pubsub_node) -> case Action(N) of -@@ -3710,8 +3550,15 @@ +@@ -3717,8 +3557,15 @@ end end, Trans). @@ -787,7 +783,7 @@ {result, Result} -> {result, Result}; {error, Error} -> {error, Error}; {atomic, {result, Result}} -> {result, Result}; -@@ -3719,6 +3566,15 @@ +@@ -3726,6 +3573,15 @@ {aborted, Reason} -> ?ERROR_MSG("transaction return internal error: ~p~n", [{aborted, Reason}]), {error, 'internal-server-error'}; @@ -803,7 +799,7 @@ {'EXIT', Reason} -> ?ERROR_MSG("transaction return internal error: ~p~n", [{'EXIT', Reason}]), {error, 'internal-server-error'}; -@@ -3727,6 +3583,16 @@ +@@ -3734,6 +3590,16 @@ {error, 'internal-server-error'} end.