[kamailio/kamailio] tm: move local-request event route to the right location to access CANCEL (PR #3979)
<!-- 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 - [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 - [x] 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: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [x] PR should be backported to stable branches - [x] Tested changes locally - [x] Related to issue #3831 #### Description <!-- Describe your changes in detail --> This PR fixes the error introduced in #3962, about not being able to access the generated CANCEL message. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3979
-- Commit Summary --
* tm: move local-request event route to the right location to access CANCEL
-- File Changes --
M src/modules/tm/t_cancel.c (93)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3979.patch https://github.com/kamailio/kamailio/pull/3979.diff
Merged #3979 into master.
Hello I tested it. The bug is fixed. BR Juan
Hello! Will this fix be included in the 5.8.4 release?
It seem it was not included in the 5.8.4 somehow. @xkaraman please check and do a cherry-pick for 5.7 and 5.8 branch.
It was not backported beacuse it was not clear for me if it goes more towards an enhancement (feature). The docs say the event route is executed for local requests that creates a transaction, however, CANCEL is associated with an INVITE transaction, it is never with a standalone purpose. It was also mentioned that it was for the case of CANCEL generated via RPC.
Overall I am not against backporting if it is considered more a bug than an enhancement, I just wanted to be sure it doesn't break expecting behaviour in a minor release. It was an email last week when the commits were backported to review if anything was missed.
Thanks for the reply. I agree that it could be considered a feature extensions or a bug fix depending on the view point.
There were two commits if I remember correctly, I thought that it might be also not cleanly apply to the stable branches. At least the first commit was about the RPC commands, which will probably not affect a lot of people. Lets have Xenofon a look at the two commits, do a quick summary and decide then.
@xkaraman commented on this pull request.
LM_DBG("event callback (%s) set, but no cfg engine\n",
+ tm_event_callback.s); + } + } + + // /* Check if msg is null */ + if(build_sip_msg_from_buf(&msg, crb->buffer, crb->buffer_len, 0) < 0) { + LM_ERR("fail to parse msg\n"); + } + + /* Call event */ + backup_rt = get_route_type(); + set_route_type(REQUEST_ROUTE); + init_run_actions_ctx(&ctx); + if(rt >= 0) { + LM_ALERT("tm:local-request found [%d]\n", rt);
This should be changed DBG.
I would say that the changes fix a bug. According to the documentation, an event should have been emitted, but it was not.
If I remember correctly, an invite was necessary for my tests before issuing the cancel request, and then issuing the RPC was another way of triggering it that did not generate the event as per the reporter.
Probably other ways of generating a locally `CANCEL` did not produce the event, but I did not verify this before.
- Backported to 5.8 (701ebefaf630cb7f6039f0030f66f736f384af69) - Backported to 5.7 (73e860525a52d8c8e07aced4b7aa6a9ee6c95865)
-
Daniel-Constantin Mierla -
Henning Westerholt
-
JuanPascual2
-
Vadim Saranov
-
Xenofon Karamanos