#### Pre-Submission Checklist - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: - [ ] PR should be backported to stable branches - [ ] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description Adding support for converting unsigned integers from database tables into kamailio pseudo variables usable in the routing config. This is quite useful when pulling `int unsigned` fields from a database, in modules that support "extra fields" in their module parameters. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3828
-- Commit Summary --
* permissions: fixup uint support in address table reload * htable: add uint support when packing an htable * lib/srdb1: add uint support for db->pv conversions
-- File Changes --
M src/lib/srdb1/db_ut.c (4) M src/modules/htable/ht_db.c (6) M src/modules/permissions/address.c (35)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3828.patch https://github.com/kamailio/kamailio/pull/3828.diff
For testing this was validated on master with the setup below.
## environment
``` root@kam:~# uname -a Linux kam 6.1.0-9-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.27-1 (2023-05-08) x86_64 GNU/Linux ```
``` root@kam:~# lsb_release -a Distributor ID: Debian Description: Debian GNU/Linux 12 (bookworm) Release: 12 Codename: bookworm ```
``` root@kam:~# kamailio -I Print out of kamailio internals Version: kamailio 5.9.0-dev1 (x86_64/linux) Default config: /usr/local/etc/kamailio/kamailio.cfg Default paths to modules: /usr/local/lib64/kamailio/modules Compile flags: USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MMAP, PKG_MALLOC, MEM_JOIN_FREE, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLOCKLIST, HAVE_RESOLV_RES, TLS_PTHREAD_MUTEX_SHARED MAX_RECV_BUFFER_SIZE=262144 MAX_SEND_BUFFER_SIZE=262144 MAX_URI_SIZE=1024 BUF_SIZE=65535 DEFAULT PKG_SIZE=8MB DEFAULT SHM_SIZE=64MB ADAPTIVE_WAIT_LOOPS=1024 TCP poll methods: poll, epoll_lt, epoll_et, sigio_rt, select Source code revision ID: unknown Compiled with: gcc 12.2.0 Compiled architecture: x86_64 Compiled on: 16:39:25 Apr 23 2024 Thank you for flying kamailio! ```
## setup
1. compile kamalio and all modules outlined in [kamailio.cfg][1] (a simplified version of a routing config with the most popular modules loaded to ensure none of them crash from the changes) 2. install mariadb-server and run `kamdbctl create` 3. add `test` table / data to kamailio DB using [test.sql][2] 4. create a self signed key/cert pair in `/usr/local/etc/kamailio` 5. configure network defines per your server in `/usr/local/etc/kamailio/kamailio.cfg` 6. run kamailio: `kamailio -P /var/run/kamailio/kamailio.pid -m 512 -M 128 -E -e -DD`
## run test
send a call via any uac to the server and watch the output from the foreground kamailio process:
``` (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:876:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] test1: -2,147,483,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:877:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] test2: 0 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:878:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] test3: 2,147,483,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:879:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] test4: 4,294,967,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:881:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (local) var(test1)=-2147483000 vn(test1)=-2147483000 avp(test1)=-2147483000 shv(test1)=-2147483000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:883:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (local) var(test2)=0 vn(test2)=0 avp(test2)=0 shv(test2)=0 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:885:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (local) var(test3)=2147483000 vn(test3)=2147483000 avp(test3)=2147483000 shv(test3)=2147483000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:887:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (local) var(test4)=-296 vn(test4)=-296 avp(test4)=-296 shv(test4)=-296 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:889:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (signed) test1: -2,147,483,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:890:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (signed) test2: 0 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:891:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (signed) test3: 2,147,483,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:892:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (unsigned) test4: 0 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:893:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (unsigned) test5: 2,147,483,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:894:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (unsigned) test6: 4,294,967,000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:896:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (db) var(test1)=-2147483000 vn(test1)=-2147483000 avp(test1)=-2147483000 shv(test1)=-2147483000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:898:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (db) var(test2)=0 vn(test2)=0 avp(test2)=0 shv(test2)=0 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:900:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (db) var(test3)=2147483000 vn(test3)=2147483000 avp(test3)=2147483000 shv(test3)=2147483000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:902:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (db) var(test4)=0 vn(test4)=0 avp(test4)=0 shv(test4)=0 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:904:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (db) var(test5)=2147483000 vn(test5)=2147483000 avp(test5)=2147483000 shv(test5)=2147483000 (65041) INFO: [/usr/local/etc/kamailio/kamailio.cfg:906:DEFAULT_ROUTE] [skpqdsqmwemdnhx@devbox1:INVITE:<null>] (db) var(test6)=4294967000 vn(test6)=4294967000 avp(test6)=4294967000 shv(test6)=4294 ```
The private variables hold the correct unsigned value with the patches applied..
[1]: https://github.com/devopsec/kamailio/blob/uint_test_files/kamailio.cfg [2]: https://github.com/devopsec/kamailio/blob/uint_test_files/test.sql
Thanks for the PR. Could you please fix the format by running clang-format on the files and do a force-push to update the individual commits? This check is right now failing.
@devopsec pushed 1 commit.
db042404783262c79271ac6fd2e0b9f1f5010822 lib/srdb1: add uint support for db->pv conversions
Thanks for the PR. Could you please fix the format by running clang-format on the files and do a force-push to update the individual commits? This check is right now failing.
Fixed the formatting, let me know if further tests need provided for this PR.
Morning Team, Checking in on this PR. Can we get this assigned?
As I get it from the patches, it adds support to permissions and htable modules. The latests versions have LONG as type for PV value, isn't it cover the unsigned int range on 64b systems (which should be the vast majority these days)?
After all, it could be merged, I was reticent when it was submitted because it takes care only of two modules and I was not sure if other modules might be impacted.
@miconda just wanted to post back real quick on this one, I think this might take a little while to review...
``` ~/projects/kamailio|[git:sr_3.1_freeze:master !]$ grep -RE -m 1 -l 'db_val2pv_spec|db1_res_t' src/modules/ | cut -d '/' -f 3 | sort -u alias_db auth_db avpops carrierroute cplc db_berkeley db_cassandra db_cluster db_mongodb db_mysql db_oracle db_perlvdb db_postgres db_redis db_sqlite db_text db_unixodbc dialog dialplan dispatcher domain domainpolicy drouting group htable imc ims_charging ims_dialog ims_icscf ims_usrloc_pcscf ims_usrloc_scscf lcr matrix mohqueue mqueue msilo mtree pdt permissions pipelimit presence presence_xml pua p_usrloc rls rtpengine rtpproxy sca secfilter speeddial sqlops topos uac uri_db userblocklist usrloc utils xcap_client xcap_server xhttp_pi ```
I have not had time to deep dive into this yet but there is probably a better way to filter down the list of modules converting those `db1_res_t` variables to pseudo variables. As far as I can tell though, even the references that are not eventually converted to PVs should be reviewed in case the logic was written without that assumption in mind. For example, I can already see in htable another case where a db_result_t in `ht_db_load_table()` is assumed to only to be an integer type and is used for indexing in the hash table (follow this variable down https://github.com/kamailio/kamailio/blob/e047f35ad5e1d7b4984a9cce72ffb782fd...).
I'll post back when I have more time to review this one in depth.
This PR is stale because it has been open 6 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.
Merged #3828 into master.
-
Daniel-Constantin Mierla -
github-actions[bot]
-
Henning Westerholt
-
Tyler Moore