[kamailio/kamailio] Rich redirects (#1243)
<!-- Kamailio Pull Request Template --> <!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception --> #### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [ ] Commits are split per component (core, individual modules, libs, utils, ...) - [ ] 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) - [x] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality) #### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [x] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number) #### Description <!-- Describe your changes in detail --> If Kamailio sends a 301/302, the current dset is added as Contact header automatically. The feature in this PR will optionally extend this Contact header with more information. - The path vector for each contact can be added as a Contact-uri Route header (rfc 3327 conformant). - The branch flags can be added as a Contact-header parameter (non-standard). In addition, the get_redirects function is extended to optionally parse the new flags parameter. The Route header can be extracted from the r-uri in the script. The new functionality must be enabled with module parameters, by default the behavior is unchanged. You can view, comment on, or merge this pull request online at: https://github.com/kamailio/kamailio/pull/1243 -- Commit Summary -- * core: Cleanup printing the dset * core: dset: Create Contacts header with path vector as Route header * core: dset: Create contacts with flags as header parameter * core modules/tm modules/sl: Make adding path and flags to redirected contacts optional * core: Increase MAX_REDIRECTION_LEN from 512 to 4096 * uac_redirect: Parse Contact's "flags" header parameter into branch flags. -- File Changes -- M src/core/config.h (2) M src/core/dset.c (252) M src/core/dset.h (6) M src/core/parser/contact/contact.c (2) M src/core/parser/contact/contact.h (1) M src/core/parser/parse_param.c (11) M src/core/parser/parse_param.h (2) M src/modules/pv/pv_core.c (2) M src/modules/sl/doc/sl_params.xml (30) M src/modules/sl/sl.c (1) M src/modules/sl/sl_funcs.c (5) M src/modules/sl/sl_funcs.h (2) M src/modules/tm/doc/params.xml (30) M src/modules/tm/t_reply.c (4) M src/modules/tm/tm.c (1) M src/modules/uac_redirect/doc/uac_redirect_admin.xml (40) M src/modules/uac_redirect/rd_funcs.c (11) M src/modules/uac_redirect/rd_funcs.h (2) M src/modules/uac_redirect/uac_redirect.c (2) M src/modules/xprint/xp_lib.c (2) -- Patch Links -- https://github.com/kamailio/kamailio/pull/1243.patch https://github.com/kamailio/kamailio/pull/1243.diff -- 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/1243
miconda commented on this pull request.
if(append_branch( 0, &scontacts[i]->uri, 0, 0, sqvalues[i],
- bflags, 0, &_redirect_sruid.uid, 0, + flags, 0, &_redirect_sruid.uid, 0, As I got it, if flags parameter in contact is set and converted successfully to integer, even `flags_hdr_mode == 2`, the bflags is not taken in consideration. Also I don't see how `flags_hdr_mode == 0` makes use of `bflags` like it was before for backward compatibility when `flags` param in contact is set. -- 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/1243#pullrequestreview-64734467
@@ -595,6 +598,17 @@ static int print_contact_str(char **dest, str *uri, qvalue_t q, str *path, char memcpy(p, buf.s, buf.len); p += buf.len; }
miconda commented on this pull request. + + /* branch flags (not SIP standard conformant) */ + buf.s = int2str(flags, &buf.len); + if (p + FLAGS_PARAM_LEN + buf.len > end) { + return -1; + } + memcpy(p, FLAGS_PARAM, FLAGS_PARAM_LEN); + p += FLAGS_PARAM_LEN; + memcpy(p, buf.s, buf.len); + p += buf.len; This should be made somehow optional, not being standard, as well as eventually do not add the flags parameter if the value is 0 (not set). -- 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/1243#pullrequestreview-64734558
@@ -595,6 +598,17 @@ static int print_contact_str(char **dest, str *uri, qvalue_t q, str *path, char memcpy(p, buf.s, buf.len); p += buf.len; }
gaaf commented on this pull request. + + /* branch flags (not SIP standard conformant) */ + buf.s = int2str(flags, &buf.len); + if (p + FLAGS_PARAM_LEN + buf.len > end) { + return -1; + } + memcpy(p, FLAGS_PARAM, FLAGS_PARAM_LEN); + p += FLAGS_PARAM_LEN; + memcpy(p, buf.s, buf.len); + p += buf.len; Patch 4 makes this optional. -- 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/1243#discussion_r140628649
@@ -595,6 +598,17 @@ static int print_contact_str(char **dest, str *uri, qvalue_t q, str *path, char memcpy(p, buf.s, buf.len); p += buf.len; }
miconda commented on this pull request. + + /* branch flags (not SIP standard conformant) */ + buf.s = int2str(flags, &buf.len); + if (p + FLAGS_PARAM_LEN + buf.len > end) { + return -1; + } + memcpy(p, FLAGS_PARAM, FLAGS_PARAM_LEN); + p += FLAGS_PARAM_LEN; + memcpy(p, buf.s, buf.len); + p += buf.len; Seems that follow up patches add the option. For the future, when doing the PR merge the patches done to the same component in one, makes the review simpler as one does it in the order they are listed and later discovers is no longer same code, being again changed. -- 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/1243#discussion_r140628661
gaaf commented on this pull request.
if(append_branch( 0, &scontacts[i]->uri, 0, 0, sqvalues[i],
- bflags, 0, &_redirect_sruid.uid, 0, + flags, 0, &_redirect_sruid.uid, 0, 1. If conversion is successful, flags are OR'ed with bflags when mode == 2. There is something wonky here, looks like an incorrectly resolved merge/rebase. I'll fix this up. 2. Good catch, i'll fix this. -- 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/1243#discussion_r140628753
@gaaf - any updates on this? Last comment from your was related to some merge/rebase errors. -- 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/1243#issuecomment-425038961
@gaaf - any updates on this?
I still have this on my todo list, just haven't found the time to fix it yet. Sorry for the delay. -- 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/1243#issuecomment-425049541
@gaaf - soft reminder just in case you can address this one and eventually have it merged. -- 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/1243#issuecomment-458240225
gaaf commented on this pull request.
if(append_branch( 0, &scontacts[i]->uri, 0, 0, sqvalues[i],
- bflags, 0, &_redirect_sruid.uid, 0, + flags, 0, &_redirect_sruid.uid, 0, Both issues were caused by the same broken merge/rebase. Solved in my new patchset. -- 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/1243#discussion_r252729680
github's broken commit sorting messes up the order of the patches, better review it as a whole. -- 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/1243#issuecomment-459401267
Worked around github's brokenness, commits are now in the correct order. -- 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/1243#issuecomment-459410615
Thanks! Looks ok after a quick review, I will go over one more try and then merge if nothing discovered. -- 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/1243#issuecomment-459461688
henningw commented on this pull request. I reviewed it as well, looks good for me. -- 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/1243#pullrequestreview-200292594
Merged #1243 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/1243#event-2131731813
@miconda - i merged it into master, if there is something else to be found we can fix it there. -- 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/1243#issuecomment-462486579
participants (3)
-
Alex Hermann -
Daniel-Constantin Mierla
-
Henning Westerholt