[sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)
Henning Westerholt
notifications at github.com
Wed Mar 31 09:33:14 CEST 2021
@henningw commented on this pull request.
Thanks for the update. I just did a quick review, focus on the usual string handling and memory allocation topics.
>
pkg_free(ptr);
- ptr = NULL;
+ *held = NULL;
+}
+
+/*
+ * lost_copy_string(str, int*) {
Have you considered using the existing ut.h pkg_str_dup(..) function?
> + /* send via connection */
+ curl = httpapi.http_connect(_m, &con, NULL, &res, mtheld, &que);
+ } else {
+ /* we have no connection ... do a NAPTR lookup */
+ if(lost_parse_host(did.s, &host, &flag) > 0) {
+
+ LM_DBG("no conn. trying NATPR lookup [%.*s]\n", host.len, host.s);
+
+ /* remove '[' and ']' from string (IPv6) */
+ if(flag == AF_INET6) {
+ host.s++;
+ host.len = host.len - 2;
+ }
+ /* is it a name or ip ... check nameinfo (reverse lookup) */
+ len = 0;
+ ipstr = lost_copy_string(host, &len);
Its necessary to make a copy here, can not the host.s be used?
> + lost_free_string(&idhdr);
+ goto err;
+ }
+ } else {
+ LM_ERR("failed to get location service for [%.*s]\n", did.len,
+ did.s);
+ lost_free_string(&que); /* clean up */
+ lost_free_string(&idhdr);
+ goto err;
+ }
+
+ LM_DBG("NATPR lookup returned [%.*s]\n", url.len, url.s);
+
+ /* curl doesn't like str */
+ len = 0;
+ lisurl = lost_copy_string(url, &len);
Its necessary to make a copy here, can not the url.s be used?
> +
+ /* clean up */
+ if(rtype != NULL) {
+ pkg_free(rtype);
+ }
+
+ if(heldreq != NULL && len == 0) {
+ LM_ERR("could not create POST request\n");
+ goto err;
+ }
+
+ LM_DBG("POST request: [%.*s]\n", len, heldreq);
+
+ /* curl doesn't like str */
+ len = 0;
+ lisurl = lost_copy_string(url, &len);
same as above
> LM_ERR("lost request failed\n");
goto err;
}
LM_DBG("findService request: [%.*s]\n", req.len, req.s);
/* send findService request to mapping server - HTTP POST */
- curlres = httpapi.http_connect(_m, &con, NULL, &ret, mtlost, &req);
+ if(naptr) {
+ /* copy url */
+ len = 0;
+ urlrep = lost_copy_string(url, &len);
same
> -
- /* get the error patterm */
- err.s = lost_get_childname(root, errors_element, &err.len);
- LM_DBG("findService error response: [%.*s]\n", err.len, err.s);
- if(err.len == 0) {
- LM_ERR("error pattern element not found: [%.*s]\n", ret.len, ret.s);
- /* free memory */
- lost_free_string(&ret);
- goto err;
+ switch(fsrdata->category) {
+ case RESPONSE:
+ if(fsrdata->uri != NULL) {
+ /* get the first uri element */
+ if((tmp.s = fsrdata->uri->value) != NULL) {
+ tmp.len = strlen(fsrdata->uri->value);
+ uri.s = lost_copy_string(tmp, &uri.len);
if you need a str anyway, you could use pkg_str_dup
> + case RESPONSE:
+ if(fsrdata->uri != NULL) {
+ /* get the first uri element */
+ if((tmp.s = fsrdata->uri->value) != NULL) {
+ tmp.len = strlen(fsrdata->uri->value);
+ uri.s = lost_copy_string(tmp, &uri.len);
+ }
+ } else {
+ LM_ERR("uri not found: [%.*s]\n", ret.len, ret.s);
+ goto err;
+ }
+ if(fsrdata->mapping != NULL) {
+ /* get the displayName element */
+ if((tmp.s = fsrdata->mapping->name->text) != NULL) {
+ tmp.len = strlen(fsrdata->mapping->name->text);
+ name.s = lost_copy_string(tmp, &name.len);
if you need a str anyway, you could use pkg_str_dup
> + tmp.len = strlen(fsrdata->mapping->name->text);
+ name.s = lost_copy_string(tmp, &name.len);
+ }
+ } else {
+ LM_ERR("name not found: [%.*s]\n", ret.len, ret.s);
+ goto err;
+ }
+ /* we are done */
+ redirect = 0;
+ break;
+ case ERROR:
+ /* get the errors element */
+ if(fsrdata->errors != NULL) {
+ if((tmp.s = fsrdata->errors->issue->type) != NULL) {
+ tmp.len = strlen(fsrdata->errors->issue->type);
+ err.s = lost_copy_string(tmp, &err.len);
if you need a str anyway, you could use pkg_str_dup
> + goto err;
+ }
+ /* clean up */
+ tmp.s = NULL;
+ tmp.len = 0;
+ /* check loop */
+ if(oldurl.s != NULL && oldurl.len > 0) {
+ if(str_strcasecmp(&url, &oldurl) == 0) {
+ LM_ERR("loop detected: "
+ "[%.*s]<-->[%.*s]\n",
+ oldurl.len, oldurl.s, url.len, url.s);
+ goto err;
+ }
+ }
+ /* remember the redirect target */
+ oldurl.s = lost_copy_string(url, &oldurl.len);
if you need a str anyway, you can use pkg_str_dup
> + if(oldurl.s != NULL && oldurl.len > 0) {
+ if(str_strcasecmp(&url, &oldurl) == 0) {
+ LM_ERR("loop detected: "
+ "[%.*s]<-->[%.*s]\n",
+ oldurl.len, oldurl.s, url.len, url.s);
+ goto err;
+ }
+ }
+ /* remember the redirect target */
+ oldurl.s = lost_copy_string(url, &oldurl.len);
+ /* clean up */
+ lost_free_findServiceResponse(&fsrdata);
+ lost_free_string(&ret);
+ /* copy url */
+ len = 0;
+ urlrep = lost_copy_string(url, &len);
same
> + if(rtp.len == 0) {
+ LM_WARN("no response type found\n");
+ rtype = NULL;
+ } else {
+ len = 0;
+ /* response type string sanity check */
+ rtype = lost_held_type(rtp.s, &exact, &len);
+ if(len == 0) {
+ LM_WARN("cannot normalize [%.*s]\n", rtp.len, rtp.s);
+ rtype = NULL;
+ }
+ }
+ }
+
+ /* default responseTime: emergencyRouting */
+ heldreq = lost_held_post_request(&len, 0, rtype);
this will allocate pkg memory to heldreq
> + len = 0;
+ /* response type string sanity check */
+ rtype = lost_held_type(rtp.s, &exact, &len);
+ if(len == 0) {
+ LM_WARN("cannot normalize [%.*s]\n", rtp.len, rtp.s);
+ rtype = NULL;
+ }
+ }
+ }
+
+ /* default responseTime: emergencyRouting */
+ heldreq = lost_held_post_request(&len, 0, rtype);
+
+ /* responseTime: milliseconds */
+ if((ltime > 0) && (strlen(ptr) == 0)) {
+ heldreq = lost_held_post_request(&len, ltime, rtype);
and this again another chunk of pkg mem to heldreq, but only one is freed later one?
> + }
+ }
+ }
+
+ /* default responseTime: emergencyRouting */
+ heldreq = lost_held_post_request(&len, 0, rtype);
+
+ /* responseTime: milliseconds */
+ if((ltime > 0) && (strlen(ptr) == 0)) {
+ heldreq = lost_held_post_request(&len, ltime, rtype);
+ }
+
+ /* responseTime: emergencyRouting|emergencyDispatch */
+ if((ltime == 0) && (strlen(ptr) > 0)) {
+ if(strncasecmp(ptr, HELD_ED, strlen(HELD_ED)) == 0) {
+ heldreq = lost_held_post_request(&len, -1, rtype);
see above
> +
+ /* default responseTime: emergencyRouting */
+ heldreq = lost_held_post_request(&len, 0, rtype);
+
+ /* responseTime: milliseconds */
+ if((ltime > 0) && (strlen(ptr) == 0)) {
+ heldreq = lost_held_post_request(&len, ltime, rtype);
+ }
+
+ /* responseTime: emergencyRouting|emergencyDispatch */
+ if((ltime == 0) && (strlen(ptr) > 0)) {
+ if(strncasecmp(ptr, HELD_ED, strlen(HELD_ED)) == 0) {
+ heldreq = lost_held_post_request(&len, -1, rtype);
+ }
+ if(strncasecmp(ptr, HELD_ER, strlen(HELD_ER)) == 0) {
+ heldreq = lost_held_post_request(&len, 0, rtype);
see above
> @@ -0,0 +1,255 @@
+/*
+ * lost module naptr functions
+ * thankfully taken over from the enum module
There are several functions (parse_naptr_regexp, naptr_greater, naptr_sort..) copied from the enum module. We could consider moving them to the core. If we want to keep them, the copyright from the author should be present here as well.
> +#define PATH_NODE (const char *)"via"
+#define PATH_NODE_VIA (const char *)"via"
+#define MAPP_NODE (const char *)"mapping"
+#define MAPP_NODE_URI (const char *)"uri"
+#define MAPP_PROP_EXP (const char *)"expires"
+#define MAPP_PROP_LUP (const char *)"lastUpdated"
+#define MAPP_PROP_SRC (const char *)"source"
+#define MAPP_PROP_SID (const char *)"sourceId"
+
+#define RED_NODE (const char *)"redirect"
+#define RED_PROP_TAR (const char *)"target"
+#define RED_PROP_SRC (const char *)"source"
+#define RED_PROP_MSG (const char *)"message"
+
+#define ERRORS_NODE (const char *)"errors"
+
The section below defines several data structures. In order to prevent overlapping with existing data structures in modules and other libraries you probably want to add a prefix (e.g. lost_list_t, lost_data_t etc..).
--
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/2675#pullrequestreview-624954455
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20210331/9cca8b17/attachment-0001.htm>
More information about the sr-dev
mailing list