Summary: | ASTERISK-24780: [patch] - Buddies are always auto-registered when processing the roster | ||
Reporter: | Simon Arlott (lp0) | Labels: | |
Date Opened: | 2015-02-11 14:01:00.000-0600 | Date Closed: | 2015-03-25 10:30:42 |
Priority: | Major | Regression? | Yes |
Status: | Closed/Complete | Components: | Resources/res_xmpp |
Versions: | 11.16.0 13.1.0 | Frequency of Occurrence | |
Related Issues: | |||
Environment: | Attachments: | ( 0) asterisk-13.1.0-24780.patch | |
Description: | In both xmpp_roster_hook and xmpp_client_create_buddy, it ignores the XMPP_AUTOREGISTER setting and sets "buddy->subscribe = 1" if there is no subscription.
This is a regression as it was already fixed in ASTERISK-14233. It is extremely inconvenient to have Asterisk send "Greetings! I am the Asterisk Open Source PBX and I want to subscribe to your presence" to everyone in your contact list. I am sharing my own XMPP account with Asterisk (at a negative priority) and this needs to be a recognised use case where Asterisk should not do inappropriate things. | ||
Comments: | By: Matt Jordan (mjordan) 2015-02-11 14:15:29.125-0600 Actually, the code in {{res_xmpp}} is already attempting to honor the autoregister feature: {code} /* Determine if we need to subscribe to their presence or not */ if (!iks_strcmp(iks_find_attrib(item, "subscription"), "none") || !iks_strcmp(iks_find_attrib(item, "subscription"), "from")) { buddy->subscribe = 1; } else { buddy->subscribe = 0; } {code} We then invoke an {{ao2_callback}} only if {{autoregister}} is enabled: {code} /* If autoregister is enabled we need to go through every buddy that we need to subscribe to and do so */ if (ast_test_flag(&clientcfg->flags, XMPP_AUTOREGISTER)) { ao2_callback(client->buddies, OBJ_NODATA | OBJ_MULTIPLE, xmpp_client_subscribe_user, client); } {code} Which actually does the subscription. Note that {{autoregister}} is on by default, and is configured in the {{general}} section. Can you attach your {{xmpp.conf}}, as well as a DEBUG log with XMPP debugging enabled? By: Simon Arlott (lp0) 2015-02-11 14:42:36.394-0600 {code} [general] debug=no ;;Turn on debugging by default. autoprune=no ;;Auto remove users from buddy list. autoregister=no ;;Auto register users from buddy list. ;autoaccept=no auth_policy=deny [asterisk] type=client serverhost=... username=... secret=... priority=-100 status=xaway statusmessage="..." {code} I can't now see where in the code Asterisk is making the subscription requests from, but I've always had autoregister=no set in the global section: ^9a6a589 jabber.conf (proxima.lp0.eu 2010-07-13 20:30:35 +0100 4) autoregister=no ;;Auto register users from buddy list. After upgrading from 1.6 there was an "autoaccept=no" line but that appears to cause the module to not load: {code} 10 19:46:39.703613 VERBOSE[8919]: config.c:2045 in config_text_file_load: Parsing '/etc/asterisk/xmpp.conf': Found 10 19:46:39.703807 ERROR[8919]: config_options.c:483 in process_category: In xmpp.conf: Processing options for general failed {code} I can't provide a DEBUG log with XMPP debugging, it'd take too long to sanitise. I've removed the offending code from res_xmpp.c. By: Simon Arlott (lp0) 2015-02-11 14:54:59.381-0600 {code} Breakpoint 2, xmpp_roster_hook (data=0xb4a37bec, pak=0x88721b4) at res_xmpp.c:2374 (gdb) p clientcfg->flags $1 = {flags = 182} {code} These flags correspond to XMPP_AUTOREGISTER | XMPP_AUTOACCEPT | XMPP_USETLS | XMPP_USESASL | XMPP_KEEPALIVE, so none of the changes to the global settings are being applied to the client settings. If I put a breakpoint at the end of xmpp_client_config_post_apply, I get the default flags: {code} (gdb) p ((struct ast_xmpp_client_config *)obj)->flags->flags $42 = 182 {code} unless I explicitly set a flag in the client section, and then: {code} (gdb) p ((struct ast_xmpp_client_config *)obj)->flags->flags $43 = 180 {code} By: Simon Arlott (lp0) 2015-02-14 10:09:35.022-0600 I can change the {{aco_option_register_custom}} parameters for the {{client_options}} that should be inherited from the {{global_options}} to remove the default value, but there doesn't appear to be a way to load the {{global_options}} first and then use them to initialise the {{client_options}}. It looks like using {{item_pre_process}} would only work if the config file had the {{general}} section first. Alternatively, the {{client_options}} would need to keep track of which flags had been modified and use that to merge them with the {{global_options}} in {{xmpp_client_config_post_apply}}. By: Simon Arlott (lp0) 2015-02-14 12:12:32.591-0600 The attached patch fixes this issue by correctly merging the XMPP_AUTO* client options with the defaults and the global options. Note: {{debug=yes}} in {{global_option}} continues to override {{debug=no}} in {{client_option}}. By: Matt Jordan (mjordan) 2015-02-16 12:18:04.733-0600 Copying over the flags in the {{xmpp_client_config_post_apply}} looks correct to me. Thanks for the patch! If you'd like this to get merged over faster, feel free to put the patch up for peer review: https://wiki.asterisk.org/wiki/display/AST/Code+Review By: Matt Jordan (mjordan) 2015-03-14 10:22:37.457-0500 I went and threw this up for code review at https://reviewboard.asterisk.org/r/4496/. While the patch is pretty straight forward, I wanted to get another pair of eyes on it. Thanks for the contribution and fixing this regression! |