[sr-dev] [kamailio/kamailio] slack: new module, send message to slack channel (#2838)

Henning Westerholt notifications at github.com
Mon Aug 30 18:38:18 CEST 2021


@henningw commented on this pull request.

Thanks for the pull request. I just had a quick look and added a few comments, related to parameter naming and error checking.

> +		return -1;
+	}
+
+	if(strncmp(val, "https://hooks.slack.com", 23)) {
+		LM_ERR("slack invalid webhook url [%s]\n", val);
+		return -1;
+	}
+
+	// TODO: parse webhook to multiple channels? eg.: chan1=>https://AAA/BBB/CC, chan2=>...
+
+	slack_url = (char*)pkg_malloc(len + 1);
+	if (slack_url==NULL) {
+		PKG_MEM_ERROR;
+		return -1;
+	}
+	strcpy(slack_url, val);

would be good to use strncpy, as you already have the len.

> +	return;
+}
+
+/**
+ * send message with curl
+ * @return 0 on success, -1 on error
+ */
+static int _curl_send(const char* uri, str *post_data)
+{
+	int datasz;
+	char* send_data;
+	CURL *curl_handle;
+	CURLcode res;
+	// LM_DBG("sending to[%s]\n", uri);
+
+	datasz = snprintf(NULL, 0, BODY_FMT, slack_channel, slack_username, post_data->s, slack_icon);

snprintf could fail, so you should check for error (negative return value).

> + */
+static int _curl_send(const char* uri, str *post_data)
+{
+	int datasz;
+	char* send_data;
+	CURL *curl_handle;
+	CURLcode res;
+	// LM_DBG("sending to[%s]\n", uri);
+
+	datasz = snprintf(NULL, 0, BODY_FMT, slack_channel, slack_username, post_data->s, slack_icon);
+	send_data = (char*)pkg_malloc((datasz+1)*sizeof(char));
+	if(send_data==NULL) {
+        LM_ERR("Error: can not allocate pkg memory [%d] bytes\n", datasz);
+        return -1;
+    }
+    snprintf(send_data, datasz+1, BODY_FMT, slack_channel, slack_username, post_data->s, slack_icon);

see above

> +	<section id="slack.p.icon_emogi">
+		<title><varname>icon_emogi</varname> (str)</title>
+		<para>
+			specify an emoji (using colon shortcodes, eg. :white_check_mark:)
+			to use as the profile photo alongside the message.
+		</para>
+		<para>
+		<emphasis>
+			Default value is :ghost:
+		</emphasis>
+		</para>
+		<example>
+		<title>Set <varname>icon_emogi</varname> parameter</title>
+		<programlisting format="linespecific">
+...
+modparam("slack", "icon_emogi", ":ghost:")

I think the parameter _icon_emogi_ is mispelled, i think it should be named _icon_emoji_. Would be good to fix this in code and docs.

> + * Exported functions
+ */
+static cmd_export_t cmds[] = {
+	{"slack_send",	  		(cmd_function)slack_send1,			1, slack_fixup,  0, ANY_ROUTE},
+	{0, 0, 0, 0, 0, 0}
+};
+
+
+/**
+ * Exported parameters
+ */
+static param_export_t mod_params[] = {
+	{ "slack_url", 			PARAM_STRING|USE_FUNC_PARAM, (void*)_slack_url_param},
+	{ "channel",			PARAM_STRING, &slack_channel }, // channel starts with #
+	{ "username",			PARAM_STRING, &slack_username },
+	{ "icon_emogi",			PARAM_STRING, &slack_icon },

I think the parameter _icon_emogi_ is mispelled, i think it should be named _icon_emoji_. Would be good to fix this in code and docs.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2838#pullrequestreview-741842887
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20210830/2cf999b4/attachment.htm>


More information about the sr-dev mailing list