[Asterisk-code-review] res calendar: Various fixes (asterisk[master])

Joshua Colp asteriskteam at digium.com
Fri Sep 15 08:20:46 CDT 2017


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/6494 )

Change subject: res_calendar: Various fixes
......................................................................

res_calendar: Various fixes

* The way that we were looking at XML elements for CalDAV was extremely
  fragile, so use SAX2 for increased robustness.

* Don't complain about a 'channel' not be specified if autoreminder is
  not set. Assume that if 'channel' is not set, we don't want to be
  notified.

* Fix some truncated CLI output in 'calendar show calendar' and make the
  'Autoreminder' description a bit more clear

ASTERISK-24588 #close
Reported by: Stefan Gofferje

ASTERISK-25523 #close
Reported by: Jesper

Change-Id: I200d11afca6a47e7d97888f286977e2e69874b2c
---
M res/res_calendar.c
M res/res_calendar_caldav.c
2 files changed, 50 insertions(+), 13 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  George Joseph: Looks good to me, approved



diff --git a/res/res_calendar.c b/res/res_calendar.c
index e4c89df..bf385db 100644
--- a/res/res_calendar.c
+++ b/res/res_calendar.c
@@ -488,6 +488,13 @@
 		}
 	}
 
+	if (cal->autoreminder && ast_strlen_zero(cal->notify_channel)) {
+		ast_log(LOG_WARNING,
+				"You have set 'autoreminder' but not 'channel' for calendar '%s.' "
+				"Notifications will not occur.\n",
+				cal->name);
+	}
+
 	if (new_calendar) {
 		cal->thread = AST_PTHREADT_NULL;
 		ast_cond_init(&cal->unload, NULL);
@@ -495,7 +502,7 @@
 		if (ast_pthread_create(&cal->thread, NULL, cal->tech->load_calendar, cal)) {
 			/* If we start failing to create threads, go ahead and return NULL
 			 * and the tech module will be unregistered
-			 */ 
+			 */
 			ao2_unlink(calendars, cal);
 			cal = unref_calendar(cal);
 		}
@@ -954,7 +961,7 @@
 	event = cmp_event ? cmp_event : old_event;
 
 	ao2_lock(event);
-	if (!cmp_event || old_event->alarm != event->alarm) {
+	if (!ast_strlen_zero(cal->notify_channel) && (!cmp_event || old_event->alarm != event->alarm)) {
 		changed = 1;
 		if (cal->autoreminder) {
 			alarm_notify_sched = (event->start - (60 * cal->autoreminder) - now.tv_sec) * 1000;
@@ -963,7 +970,7 @@
 		}
 
 		/* For now, send the notification if we missed it, but the meeting hasn't happened yet */
-		if (event->start >=  now.tv_sec) {
+		if (event->start >= now.tv_sec) {
 			if (alarm_notify_sched <= 0) {
 				alarm_notify_sched = 1;
 			}
@@ -1596,7 +1603,7 @@
 
 static char *handle_show_calendar(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
-#define FORMAT "%-17.17s : %-20.20s\n"
+#define FORMAT  "%-18.18s : %-20.20s\n"
 #define FORMAT2 "%-12.12s: %-40.60s\n"
 	struct ao2_iterator i;
 	struct ast_calendar *cal;
@@ -1645,7 +1652,13 @@
 	ast_cli(a->fd, FORMAT, "Notify appdata", cal->notify_appdata);
 	ast_cli(a->fd, "%-17.17s : %d\n", "Refresh time", cal->refresh);
 	ast_cli(a->fd, "%-17.17s : %d\n", "Timeframe", cal->timeframe);
-	ast_cli(a->fd, "%-17.17s : %d\n", "Autoreminder", cal->autoreminder);
+
+	if (cal->autoreminder) {
+		ast_cli(a->fd, "%-17.17s : %d minutes before event\n", "Autoreminder", cal->autoreminder);
+	} else {
+		ast_cli(a->fd, "%-17.17s : None\n", "Autoreminder");
+	}
+
 	ast_cli(a->fd, "%s\n", "Events");
 	ast_cli(a->fd, "%s\n", "------");
 
diff --git a/res/res_calendar_caldav.c b/res/res_calendar_caldav.c
index 3c1c74a..b6822b0 100644
--- a/res/res_calendar_caldav.c
+++ b/res/res_calendar_caldav.c
@@ -156,6 +156,7 @@
 	ne_add_response_body_reader(req, debug_response_handler, fetch_response_reader, &response);
 	ne_set_request_body_buffer(req, ast_str_buffer(req_body), ast_str_strlen(req_body));
 	ne_add_request_header(req, "Content-type", ast_strlen_zero(content_type) ? "text/xml" : content_type);
+	ne_add_request_header(req, "Depth", "1");
 
 	ret = ne_request_dispatch(req);
 	ne_request_destroy(req);
@@ -476,17 +477,26 @@
 	time_t end;
 };
 
-static void handle_start_element(void *data, const xmlChar *fullname, const xmlChar **atts)
+static const xmlChar *caldav_node_localname = BAD_CAST "calendar-data";
+static const xmlChar *caldav_node_nsuri     = BAD_CAST "urn:ietf:params:xml:ns:caldav";
+
+static void handle_start_element(void *data,
+								 const xmlChar *localname, const xmlChar *prefix, const xmlChar *uri,
+								 int nb_namespaces, const xmlChar **namespaces,
+								 int nb_attributes, int nb_defaulted, const xmlChar **attributes)
 {
 	struct xmlstate *state = data;
 
-	if (!xmlStrcasecmp(fullname, BAD_CAST "C:calendar-data") || !xmlStrcasecmp(fullname, BAD_CAST "caldav:calendar-data")) {
-		state->in_caldata = 1;
-		ast_str_reset(state->cdata);
+	if (xmlStrcmp(localname, caldav_node_localname) || xmlStrcmp(uri, caldav_node_nsuri)) {
+		return;
 	}
+
+	state->in_caldata = 1;
+	ast_str_reset(state->cdata);
 }
 
-static void handle_end_element(void *data, const xmlChar *name)
+static void handle_end_element(void *data,
+							   const xmlChar *localname, const xmlChar *prefix, const xmlChar *uri)
 {
 	struct xmlstate *state = data;
 	struct icaltimetype start, end;
@@ -494,7 +504,7 @@
 	icalcomponent *iter;
 	icalcomponent *comp;
 
-	if (xmlStrcasecmp(name, BAD_CAST "C:calendar-data") && xmlStrcasecmp(name, BAD_CAST "caldav:calendar-data")) {
+	if (xmlStrcmp(localname, caldav_node_localname) || xmlStrcmp(uri, caldav_node_nsuri)) {
 		return;
 	}
 
@@ -557,9 +567,23 @@
 	state.start = start;
 	state.end = end;
 
+	/*
+	 * We want SAX2, so you assume that we want to call xmlSAXVersion() here, and
+	 * that certainly seems like the right thing to do, but the default SAX
+	 * handling functions assume that the 'data' pointer is going to be a
+	 * xmlParserCtxtPtr, not a user data pointer, so we have to make sure that we
+	 * are only calling the handlers that we control.
+	 *
+	 * So instead we hack things up a bit, clearing the struct and then assigning
+	 * the magic number manually.
+	 *
+	 * There may be a cleaner way to do this, but frankly the libxml2 docs are
+	 * pretty sparse.
+	 */
 	memset(&saxHandler, 0, sizeof(saxHandler));
-	saxHandler.startElement = handle_start_element;
-	saxHandler.endElement = handle_end_element;
+	saxHandler.initialized = XML_SAX2_MAGIC;
+	saxHandler.startElementNs = handle_start_element;
+	saxHandler.endElementNs = handle_end_element;
 	saxHandler.characters = handle_characters;
 
 	xmlSAXUserParseMemory(&saxHandler, &state, ast_str_buffer(response), ast_str_strlen(response));

-- 
To view, visit https://gerrit.asterisk.org/6494
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I200d11afca6a47e7d97888f286977e2e69874b2c
Gerrit-Change-Number: 6494
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170915/187f4496/attachment.html>


More information about the asterisk-code-review mailing list