Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Merged #2675 into master. -- 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#event-4574674561___ Kamailio (SER) - Development Mailing Lis

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Merge it with the patch, it is only changing the printing of the log message in that case and should be legit that the header is missing, not being mandatory. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.co

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> > > yes, have checked "ERROR" generation. > This happens because `part_multipart_headers_cmp()` try search content by > "Content-Id". > The first content element (SDP in the example) does not have "Content-Id". > Function reaches the end of the content element and generates the error. > > The

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

yes, have checked "ERROR" generation. This happens because `part_multipart_headers_cmp()` try search content by "Content-Id". The first content element (SDP in the example) does not have "Content-Id". Function reaches the end of the content element and generates the error. Then `part_multipart_he

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich pushed 1 commit. 5f1959cd42ff925009dd9e0fdd87f9d0487ec4fe lost: typedef naming changed and copyright added -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/2675/files/106b8a659bd4a53e8a6872e9db8a9af0f12

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich, @sergey-safarov: the error message from part_multipart_headers_cmp() does not seem related to the code added in this PR, so, even if it is an issue, it is not required to be fixed by this PR. If proves to be a bug, an issue can be opened, or even better, a new PR can be made to fix it

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> @wkampich Thanks for the update. I think the comments regarding the missing > original copyright notice in the natptr.[c,h] files and also the response.h > naming are not resolved yet. The copyright notice should be added, I can also > do it later if you prefer this. > Regarding the naming com

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> Hello, @wkampich > Let me review the change about ERROR waring. > I want to try to understand in detail why the error generated. Maybe I can > suggest another approach. > Please give today and tomorrow. @sergey-safarov: yes, ok for me - just a hint: the problem is not in the multipart header p

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Hell @wkampich let me review the change about ERROR waring. I want to try to understand in detail why the error generated. Maybe I can suggest another approach. Please give today and tommorow. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or vi

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich Thanks for the update. I think the comments regarding the missing original copyright notice in the natptr.[c,h] files and also the response.h naming are not resolved yet. The copyright notice should be added, I can also do it later if you prefer this. Regarding the naming comment, I ju

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Thank you for your contributions, Wolfgang! -- 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#issuecomment-815548745___ Kamailio (SER) -

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

If there are no further comments, I will merge the pull request today (late afternoon). Just a note about "handling more than one `Geolocation` header": The LoST module creates a list of `Geolocation` headers (be it multiple values in a header or single headers) and can decide which header type t

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich pushed 1 commit. 106b8a659bd4a53e8a6872e9db8a9af0f12ff70a core: parse_body.c/part_multipart_headers_cmp() changed log output from ERR to DBG in case a header does not exist -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/ka

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

If it is a valid multipart body, with a header that may not exist, then the log message should be DEBUG. You can make the patch if that's the case. -- 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/

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> Regarding: > > ``` > 1(82) ERROR: [core/parser/parse_body.c:510]: > part_multipart_headers_cmp(): error code: "-9" error text: "We reached the > end of the buffer". > ``` > > Maybe there are white spaces at the end of body that fit inside the > Content-Length in the sipp scenario (try to p

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Regarding: ``` 1(82) ERROR: [core/parser/parse_body.c:510]: part_multipart_headers_cmp(): error code: "-9" error text: "We reached the end of the buffer". ``` Maybe there are white spaces at the end of body that fit inside the Content-Length in the sipp scenario (try to print the $mb enclose

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich pushed 1 commit. 01fa6d962258a7ea82e6d3fe2d56f98b94806a31 lost: removed some typos and added explanatory text to doc -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/2675/files/b458bbcba00cd6678617f004b

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> tested config > > ``` > listen=udp:127.0.0.1:5060 > loadmodule "xlog.so" > loadmodule "pv.so" > loadmodule "http_client.so" > loadmodule "lost.so" > loadmodule "textopsx.so" > > modparam("lost", "exact_type", 1) > modparam("lost", "post_request", 1) > modparam("http_client", "httpcon", > "lost

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> tested this config file > > ``` > listen=udp:127.0.0.1:5060 > loadmodule "xlog.so" > loadmodule "pv.so" > loadmodule "http_client.so" > loadmodule "lost.so" > loadmodule "textopsx.so" > > modparam("lost", "exact_type", 1) > > request_route { > $var(geo_uri) = @hf_value.geolocation[1].uri; >

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + + + The return value is 200 on success, 400 if an internal error occured, or 500 if an +error code is returned in the HELD response. + +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> Use case when the call contains two `Geolocation` headers. Related > `geoheader_type` param. > > Example > > ``` > Geolocation: > Geolocation: > ``` > > Technically such a call contains Geolocacation by_value and does not require > deference first Geolocation

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Use case when the call contains two `Geolocation` headers. Example ``` Geolocation: Geolocation: ``` Technically such a call contains Geolocacation by_value and does not require deference first Geolocation header by_reference. >From my point of view. When used `

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

tested config ``` listen=udp:127.0.0.1:5060 loadmodule "xlog.so" loadmodule "pv.so" loadmodule "http_client.so" loadmodule "lost.so" loadmodule "textopsx.so" modparam("lost", "exact_type", 1) modparam("lost", "post_request", 1) modparam("http_client", "httpcon", "lostsrv=>http://psap.or.nga911.co

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@miconda commented on this pull request. > + + + The return value is 200 on success, 400 if an internal error occured, or 500 if an +error code is returned in the HELD response. + +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Tested config ``` listen=udp:127.0.0.1:5060 loadmodule "xlog.so" loadmodule "pv.so" loadmodule "http_client.so" loadmodule "lost.so" loadmodule "textopsx.so" modparam("lost", "exact_type", 1) modparam("lost", "post_request", 1) request_route { $var(id) = "sip:al...@ca.nga911.com"; $var(res) =

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@sergey-safarov commented on this pull request. > @@ -168,7 +176,59 @@ Set location_type parameter ... -modparam("lost", "location_type, "civic geodetic locationURI") +modparam("lost", "location_type", "civic geodetic locationURI") +... +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

tested this config file ``` listen=udp:127.0.0.1:5060 loadmodule "xlog.so" loadmodule "pv.so" loadmodule "http_client.so" loadmodule "lost.so" loadmodule "textopsx.so" modparam("lost", "exact_type", 1) request_route { $var(geo_uri) = @hf_value.geolocation[1].uri; if ($(var(geo_uri){s.select,0

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@sergey-safarov commented on this pull request. > + + + The return value is 200 on success, 400 if an internal error occured, or 500 if an +error code is returned in the HELD response. + +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich pushed 1 commit. b458bbcba00cd6678617f004bbdeff2a1b0c524d lost: memory leak fix and code refactoring -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/2675/files/25378c0388bb4365e8d4f1a4e874381fe339cd3b.

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > +#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 *)

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich: lost_copy_string() is not a significant amount of code to replace with pkg_str_dup(), the signature is different as well -- overall, it could be good, but not a must. If you fix the leak, the PR should be ready to merge. -- You are receiving this because you are subscribed to this th

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + goto err; + } + /* clean up */ + tmp.s = NULL; +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + tmp.len = > strlen(fsrdata->mapping->name->text); + name.s = lost_copy_string(tmp, &name.len); + } +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + case RESPONSE: + if(fsrdata->uri != NULL) { + /* get the first uri element */ + if((tmp.s = fsrdata->uri->value) != NULL) {

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > - - /* 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) { -

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + if(rtp.len == 0) { + LM_WARN("no response type found\n"); + rtype = NULL; + } else { + len = 0; + /* response type string sanity check *

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + if(oldurl.s != NULL && > oldurl.len > 0) { + if(str_strcasecmp(&url, &oldurl) == 0) { +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > 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_

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + + /* 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(

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + lost_free_string(&idhdr); + goto err; + } + } else { + LM_ERR("failed to get location service for [%.*s]\n", did.len, +

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > + /* 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,

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich commented on this pull request. > pkg_free(ptr); - ptr = NULL; + *held = NULL; +} + +/* + * lost_copy_string(str, int*) { yes true `pkg_str_dup(..)` would be an option; basically I only use the function when a pointer to a string is necessary and requires zero t

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Good point @miconda regarding the zero termination, then of course one needs to do a copy. The license topic is of course valid as well, the original author of the enum code is Juha. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Moving code to the core from existing module must ensure it can be relocated under BSD license. Regarding the cloning of some str values, they seem to be required due to used functions that require zero-terminated strings, like ipstr that is passed to lost_get_nameinfo(), which is using libc in

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@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

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

All good. Thanks for fixing! -- 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#issuecomment-810817098___ Kamailio (SER) - Development Ma

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Code duplication was removed for http client module. I guess this can be merged, if no other opinions against. -- 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#issuecomment-81080

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich pushed 3 commits. 2f48d0cdc84eaa583a2f2b6837e6251e62abc901 lost: DOM level count fix 5f50cc0ec7017a325d25249e6e66e6a2feb4dd08 http_client: duplicated code removed 25378c0388bb4365e8d4f1a4e874381fe339cd3b lost: README update -- You are receiving this because you are subscribed to th

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> > Please don't create that much duplicated code. Make the old API function > > call the new as a wrapper with a null argument for content type header. > > That way we keep backwards compatibility and can add new functions. > > Please also add new tests to the API test module. > > Hi Olle, i th

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

> Please don't create that much duplicated code. Make the old API function call > the new as a wrapper with a null argument for content type header. That way > we keep backwards compatibility and can add new functions. > > Please also add new tests to the API test module. Hi Olle, i think you a

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

@wkampich pushed 1 commit. ffb0a8624af952e619ddc5f6fa29c6655f17d4c9 lost: bug-fix due to a code formatting error -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/2675/files/356fba18ac5eef25e67a2357b4cdcd925581009

Re: [sr-dev] [kamailio/kamailio] extensions to lost and http_client modules (#2675)

Please don't create that much duplicated code. Make the old API function call the new as a wrapper with a null argument for content type header. That way we keep backwards compatibility and can add new functions. Please also add new tests to the API test module. -- You are receiving this becau